Skip to content

Actions: fix filtering of code injection results between medium and critical version of query #20937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Actions: fix filtering of code injection results between medium and critical version of query #20937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
647b928
Add test with push and workflow_dispatch triggers
owen-mc Nov 28, 2025
6a1a1b0
Fix filtering of code injection alerts between medium and critical
owen-mc Nov 28, 2025
6d3ee98
Add change note
owen-mc Nov 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions actions/ql/lib/change-notes/2025-11-28-fix-code-injection-alert-filtering.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: majorAnalysis
---
* 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`.
22 changes: 22 additions & 0 deletions actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,25 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig {

/** Tracks flow of unsafe user input that is used to construct and evaluate a code script. */
module CodeInjectionFlow = TaintTracking::Global<CodeInjectionConfig>;

/**
* Holds if the flow from `source` to `sink` has critical severity and they are
* linked by `event`.
*/
pragma[inline]
predicate criticalSeverity(DataFlow::Node source, DataFlow::Node sink, Event event) {
event = getRelevantCriticalEventForSink(sink) and
source.(RemoteFlowSource).getEventName() = event.getName()
}

/** Holds if the flow from `source` to `sink` has medium severity. */
pragma[inline]
predicate mediumSeverity(DataFlow::Node source, DataFlow::Node sink) {
not criticalSeverity(source, sink, _) and
// exclude cases where the sink is a JS script and the expression uses toJson
not exists(UsesStep script |
script.getCallee() = "actions/github-script" and
script.getArgumentExpr("script") = sink.asExpr() and
exists(getAToJsonReferenceExpression(sink.asExpr().(Expression).getExpression(), _))
)
}
3 changes: 1 addition & 2 deletions actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import codeql.actions.security.ControlChecks
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event
where
CodeInjectionFlow::flowPath(source, sink) and
event = getRelevantCriticalEventForSink(sink.getNode()) and
source.getNode().(RemoteFlowSource).getEventName() = event.getName()
criticalSeverity(source.getNode(), sink.getNode(), event)
select sink.getNode(), source, sink,
"Potential code injection in $@, which may be controlled by an external user ($@).", sink,
sink.getNode().asExpr().(Expression).getRawExpression(), event, event.getName()
8 changes: 1 addition & 7 deletions actions/ql/src/Security/CWE-094/CodeInjectionMedium.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,7 @@ import CodeInjectionFlow::PathGraph
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
where
CodeInjectionFlow::flowPath(source, sink) and
inNonPrivilegedContext(sink.getNode().asExpr()) and
// exclude cases where the sink is a JS script and the expression uses toJson
not exists(UsesStep script |
script.getCallee() = "actions/github-script" and
script.getArgumentExpr("script") = sink.getNode().asExpr() and
exists(getAToJsonReferenceExpression(sink.getNode().asExpr().(Expression).getExpression(), _))
)
mediumSeverity(source.getNode(), sink.getNode())
select sink.getNode(), source, sink,
"Potential code injection in $@, which may be controlled by an external user.", sink,
sink.getNode().asExpr().(Expression).getRawExpression()
18 changes: 18 additions & 0 deletions ...ons/ql/test/query-tests/Security/CWE-094/.github/workflows/push_and_workflow_dispatch.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
on:
push:
workflow_dispatch:

jobs:
echo-chamber:
runs-on: ubuntu-latest
steps:
- run: echo '${{ github.event.commits[11].message }}'
- run: echo '${{ github.event.commits[11].author.email }}'
- run: echo '${{ github.event.commits[11].author.name }}'
- run: echo '${{ github.event.head_commit.message }}'
- run: echo '${{ github.event.head_commit.author.email }}'
- run: echo '${{ github.event.head_commit.author.name }}'
- run: echo '${{ github.event.head_commit.committer.email }}'
- run: echo '${{ github.event.head_commit.committer.name }}'
- run: echo '${{ github.event.commits[11].committer.email }}'
- run: echo '${{ github.event.commits[11].committer.name }}'
10 changes: 10 additions & 0 deletions actions/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,16 @@ nodes
| .github/workflows/push.yml:14:19:14:64 | github.event.head_commit.committer.name | semmle.label | github.event.head_commit.committer.name |
| .github/workflows/push.yml:15:19:15:65 | github.event.commits[11].committer.email | semmle.label | github.event.commits[11].committer.email |
| .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | semmle.label | github.event.commits[11].committer.name |
| .github/workflows/push_and_workflow_dispatch.yml:9:19:9:57 | github.event.commits[11].message | semmle.label | github.event.commits[11].message |
| .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 |
| .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 |
| .github/workflows/push_and_workflow_dispatch.yml:12:19:12:57 | github.event.head_commit.message | semmle.label | github.event.head_commit.message |
| .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 |
| .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 |
| .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 |
| .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 |
| .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 |
| .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 |
| .github/workflows/reusable-workflow-1.yml:6:7:6:11 | input taint | semmle.label | input taint |
| .github/workflows/reusable-workflow-1.yml:36:21:36:39 | inputs.taint | semmle.label | inputs.taint |
| .github/workflows/reusable-workflow-1.yml:44:19:44:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
Expand Down
Loading