Skip to content

Commit fafb44d

Browse files
author
Alvaro Muñoz
committed
Add CachePoisoning by Code Injection query
1 parent b965a55 commit fafb44d

File tree

6 files changed

+84
-5
lines changed

6 files changed

+84
-5
lines changed

‎ql/lib/codeql/actions/security/CachePoisoningQuery.qll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import actions
22

3+
string defaultBranchTriggerEvent() {
4+
result =
5+
[
6+
"check_run", "check_suite", "delete", "discussion", "discussion_comment", "fork", "gollum",
7+
"issue_comment", "issues", "label", "milestone", "project", "project_card", "project_column",
8+
"public", "pull_request_comment", "pull_request_target", "repository_dispatch", "schedule",
9+
"watch", "workflow_run"
10+
]
11+
}
12+
313
abstract class CacheWritingStep extends Step { }
414

515
class CacheActionUsesStep extends CacheWritingStep, UsesStep {
@@ -60,3 +70,10 @@ class SetupDotnetUsesStep extends CacheWritingStep, UsesStep {
6070
)
6171
}
6272
}
73+
74+
class SetupRubyUsesStep extends CacheWritingStep, UsesStep {
75+
SetupRubyUsesStep() {
76+
this.getCallee() = ["actions/setup-ruby", "ruby/setup-ruby"] and
77+
this.getArgument("bundler-cache") = "true"
78+
}
79+
}

‎ql/src/Security/CWE-349/CachePoisoningByCodeInjection.ql

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* @name Cache Poisoning via low-privilege code injection
3+
* @description The cache can be poisoned by untrusted code, leading to a cache poisoning attack.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @security-severity 9.3
8+
* @id actions/cache-poisoning/code-injection
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-349
12+
* external/cwe/cwe-094
13+
*/
14+
15+
import actions
16+
import codeql.actions.security.CodeInjectionQuery
17+
import codeql.actions.security.CachePoisoningQuery
18+
import CodeInjectionFlow::PathGraph
19+
20+
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, LocalJob j
21+
where
22+
CodeInjectionFlow::flowPath(source, sink) and
23+
j = sink.getNode().asExpr().getEnclosingJob() and
24+
not j.isPrivileged() and
25+
// The workflow runs in the context of the default branch
26+
// TODO: (require to collect trigger types)
27+
// - add push to default branch?
28+
// - exclude pull_request_target when branches_ignore includes default branch or when branches does not include the default branch
29+
(
30+
j.getEnclosingWorkflow().hasTriggerEvent(defaultBranchTriggerEvent())
31+
or
32+
j.getEnclosingWorkflow().hasTriggerEvent("workflow_call") and
33+
exists(ExternalJob call, Workflow caller |
34+
call.getCallee() = j.getLocation().getFile().getRelativePath() and
35+
caller = call.getWorkflow() and
36+
caller.hasTriggerEvent(defaultBranchTriggerEvent())
37+
)
38+
)
39+
select sink.getNode(), source, sink,
40+
"Potential code injection in $@, which may be controlled by an external user.", sink,
41+
sink.getNode().asExpr().(Expression).getRawExpression()

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test9.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
on:
2+
issue_comment:
3+
types: [created]
4+
5+
jobs:
6+
pr-comment:
7+
runs-on: ubuntu-latest
8+
permissions: {}
9+
steps:
10+
- run: |
11+
echo ${{ github.event.comment.body }}
12+

‎ql/test/query-tests/Security/CWE-349/CachePoisoning.expected

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
| .github/workflows/test1.yml:17:9:21:6 | Uses Step | Potential cache poisoning on privileged workflow. |
2-
| .github/workflows/test2.yml:12:9:16:6 | Uses Step | Potential cache poisoning on privileged workflow. |
3-
| .github/workflows/test3.yml:12:9:20:6 | Uses Step | Potential cache poisoning on privileged workflow. |
4-
| .github/workflows/test6.yml:12:9:16:6 | Uses Step | Potential cache poisoning on privileged workflow. |
5-
| .github/workflows/test7.yml:12:9:15:6 | Uses Step | Potential cache poisoning on privileged workflow. |
1+
| .github/workflows/test1.yml:12:9:17:6 | Uses Step | Potential cache poisoning on privileged workflow. |
2+
| .github/workflows/test2.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |
3+
| .github/workflows/test3.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |
4+
| .github/workflows/test6.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |
5+
| .github/workflows/test7.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |
6+
| .github/workflows/test8.yml:12:9:17:6 | Uses Step | Potential cache poisoning on privileged workflow. |

‎ql/test/query-tests/Security/CWE-349/CachePoisoningByCodeInjection.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
edges
2+
nodes
3+
| .github/workflows/test9.yml:11:17:11:48 | github.event.comment.body | semmle.label | github.event.comment.body |
4+
subpaths
5+
#select
6+
| .github/workflows/test9.yml:11:17:11:48 | github.event.comment.body | .github/workflows/test9.yml:11:17:11:48 | github.event.comment.body | .github/workflows/test9.yml:11:17:11:48 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:11:17:11:48 | github.event.comment.body | ${{ github.event.comment.body }} |

‎ql/test/query-tests/Security/CWE-349/CachePoisoningByCodeInjection.qlref

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Security/CWE-349/CachePoisoningByCodeInjection.ql
2+

0 commit comments

Comments
 (0)