Skip to content

Commit 6d301f2

Browse files
owen-mcowen-mc
authored
Merge pull request #20937 from owen-mc/actions/fix/code-injection-privileged-context
Actions: fix filtering of code injection results between medium and critical version of query
2 parents d70c596 + 4a16de2 commit 6d301f2

File tree

7 files changed

+94
-19
lines changed

7 files changed

+94
-19
lines changed

‎actions/ql/lib/change-notes/2025-11-28-fix-code-injection-alert-filtering.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* The query `actions/code-injection/medium` has been updated to include results which were incorrectly excluded while filtering out results that are reported by `actions/code-injection/critical`.

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

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,7 @@ class CodeInjectionSink extends DataFlow::Node {
1919
Event getRelevantCriticalEventForSink(DataFlow::Node sink) {
2020
inPrivilegedContext(sink.asExpr(), result) and
2121
not exists(ControlCheck check | check.protects(sink.asExpr(), result, "code-injection")) and
22-
// exclude cases where the sink is a JS script and the expression uses toJson
23-
not exists(UsesStep script |
24-
script.getCallee() = "actions/github-script" and
25-
script.getArgumentExpr("script") = sink.asExpr() and
26-
exists(getAToJsonReferenceExpression(sink.asExpr().(Expression).getExpression(), _))
27-
)
22+
not isGithubScriptUsingToJson(sink.asExpr())
2823
}
2924

3025
/**
@@ -91,3 +86,38 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig {
9186

9287
/** Tracks flow of unsafe user input that is used to construct and evaluate a code script. */
9388
module CodeInjectionFlow = TaintTracking::Global<CodeInjectionConfig>;
89+
90+
/**
91+
* Holds if there is a code injection flow from `source` to `sink` with
92+
* critical severity, linked by `event`.
93+
*/
94+
predicate criticalSeverityCodeInjection(
95+
CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event
96+
) {
97+
CodeInjectionFlow::flowPath(source, sink) and
98+
event = getRelevantCriticalEventForSink(sink.getNode()) and
99+
source.getNode().(RemoteFlowSource).getEventName() = event.getName()
100+
}
101+
102+
/**
103+
* Holds if there is a code injection flow from `source` to `sink` with medium severity.
104+
*/
105+
predicate mediumSeverityCodeInjection(
106+
CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
107+
) {
108+
CodeInjectionFlow::flowPath(source, sink) and
109+
not criticalSeverityCodeInjection(source, sink, _) and
110+
not isGithubScriptUsingToJson(sink.getNode().asExpr())
111+
}
112+
113+
/**
114+
* Holds if `expr` is the `script` input to `actions/github-script` and it uses
115+
* `toJson`.
116+
*/
117+
predicate isGithubScriptUsingToJson(Expression expr) {
118+
exists(UsesStep script |
119+
script.getCallee() = "actions/github-script" and
120+
script.getArgumentExpr("script") = expr and
121+
exists(getAToJsonReferenceExpression(expr.getExpression(), _))
122+
)
123+
}

‎actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ import CodeInjectionFlow::PathGraph
2020
import codeql.actions.security.ControlChecks
2121

2222
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event
23-
where
24-
CodeInjectionFlow::flowPath(source, sink) and
25-
event = getRelevantCriticalEventForSink(sink.getNode()) and
26-
source.getNode().(RemoteFlowSource).getEventName() = event.getName()
23+
where criticalSeverityCodeInjection(source, sink, event)
2724
select sink.getNode(), source, sink,
2825
"Potential code injection in $@, which may be controlled by an external user ($@).", sink,
2926
sink.getNode().asExpr().(Expression).getRawExpression(), event, event.getName()

‎actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,7 @@ import codeql.actions.security.CodeInjectionQuery
1919
import CodeInjectionFlow::PathGraph
2020

2121
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
22-
where
23-
CodeInjectionFlow::flowPath(source, sink) and
24-
inNonPrivilegedContext(sink.getNode().asExpr()) and
25-
// exclude cases where the sink is a JS script and the expression uses toJson
26-
not exists(UsesStep script |
27-
script.getCallee() = "actions/github-script" and
28-
script.getArgumentExpr("script") = sink.getNode().asExpr() and
29-
exists(getAToJsonReferenceExpression(sink.getNode().asExpr().(Expression).getExpression(), _))
30-
)
22+
where mediumSeverityCodeInjection(source, sink)
3123
select sink.getNode(), source, sink,
3224
"Potential code injection in $@, which may be controlled by an external user.", sink,
3325
sink.getNode().asExpr().(Expression).getRawExpression()

‎actions/ql/test/query-tests/Security/CWE-094/.github/workflows/push_and_workflow_dispatch.yml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
on:
2+
push:
3+
workflow_dispatch:
4+
5+
jobs:
6+
echo-chamber:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- run: echo '${{ github.event.commits[11].message }}'
10+
- run: echo '${{ github.event.commits[11].author.email }}'
11+
- run: echo '${{ github.event.commits[11].author.name }}'
12+
- run: echo '${{ github.event.head_commit.message }}'
13+
- run: echo '${{ github.event.head_commit.author.email }}'
14+
- run: echo '${{ github.event.head_commit.author.name }}'
15+
- run: echo '${{ github.event.head_commit.committer.email }}'
16+
- run: echo '${{ github.event.head_commit.committer.name }}'
17+
- run: echo '${{ github.event.commits[11].committer.email }}'
18+
- run: echo '${{ github.event.commits[11].committer.name }}'

‎actions/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,16 @@ nodes
435435
| .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | semmle.label | github.event.head_commit.committer.name |
436436
| .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | semmle.label | github.event.commits[11].committer.email |
437437
| .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | semmle.label | github.event.commits[11].committer.name |
438+
| .github/workflows/push_and_workflow_dispatch.yml:9:19:9:57 | github.event.commits[11].message | semmle.label | github.event.commits[11].message |
439+
| .github/workflows/push_and_workflow_dispatch.yml:10:19:10:62 | github.event.commits[11].author.email | semmle.label | github.event.commits[11].author.email |
440+
| .github/workflows/push_and_workflow_dispatch.yml:11:19:11:61 | github.event.commits[11].author.name | semmle.label | github.event.commits[11].author.name |
441+
| .github/workflows/push_and_workflow_dispatch.yml:12:19:12:57 | github.event.head_commit.message | semmle.label | github.event.head_commit.message |
442+
| .github/workflows/push_and_workflow_dispatch.yml:13:19:13:62 | github.event.head_commit.author.email | semmle.label | github.event.head_commit.author.email |
443+
| .github/workflows/push_and_workflow_dispatch.yml:14:19:14:61 | github.event.head_commit.author.name | semmle.label | github.event.head_commit.author.name |
444+
| .github/workflows/push_and_workflow_dispatch.yml:15:19:15:65 | github.event.head_commit.committer.email | semmle.label | github.event.head_commit.committer.email |
445+
| .github/workflows/push_and_workflow_dispatch.yml:16:19:16:64 | github.event.head_commit.committer.name | semmle.label | github.event.head_commit.committer.name |
446+
| .github/workflows/push_and_workflow_dispatch.yml:17:19:17:65 | github.event.commits[11].committer.email | semmle.label | github.event.commits[11].committer.email |
447+
| .github/workflows/push_and_workflow_dispatch.yml:18:19:18:64 | github.event.commits[11].committer.name | semmle.label | github.event.commits[11].committer.name |
438448
| .github/workflows/reusable-workflow-1.yml:6:7:6:11 | input taint | semmle.label | input taint |
439449
| .github/workflows/reusable-workflow-1.yml:36:21:36:39 | inputs.taint | semmle.label | inputs.taint |
440450
| .github/workflows/reusable-workflow-1.yml:44:19:44:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |

0 commit comments

Comments
 (0)