Skip to content

Commit b796077

Browse files
author
Alvaro Muñoz
authored
Merge pull request #7 from github/fix_dorny_paths_filter_source
Fix incorrect source for dorny path filters
2 parents b22e305 + ddf4bb1 commit b796077

File tree

7 files changed

+114
-8
lines changed

7 files changed

+114
-8
lines changed

‎ql/lib/codeql/actions/dataflow/FlowSources.qll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,25 @@ class CompositeActionInputSource extends RemoteFlowSource {
252252
}
253253

254254
/**
255-
* A downloadeded artifact.
255+
* A downloaded artifact.
256256
*/
257257
private class ArtifactSource extends RemoteFlowSource {
258258
ArtifactSource() { this.asExpr() instanceof UntrustedArtifactDownloadStep }
259259

260260
override string getSourceType() { result = "artifact" }
261261
}
262+
263+
/**
264+
* A list of file names returned by dorny/paths-filter.
265+
*/
266+
private class DornyPathsFilterSource extends RemoteFlowSource {
267+
DornyPathsFilterSource() {
268+
exists(UsesStep u |
269+
u.getCallee() = "dorny/paths-filter" and
270+
u.getArgument("list-files") = ["csv", "json"] and
271+
this.asExpr() = u
272+
)
273+
}
274+
275+
override string getSourceType() { result = "filename" }
276+
}

‎ql/lib/codeql/actions/dataflow/FlowSteps.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,23 @@ class ArtifactDownloadToUseTaintStep extends AdditionalTaintStep {
124124
artifactDownloadToUseStep(node1, node2)
125125
}
126126
}
127+
128+
/**
129+
* A read of the _files field of the dorny/paths-filter action.
130+
*/
131+
predicate dornyPathsFilterTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
132+
exists(UsesStep u, StepsExpression o |
133+
u.getCallee() = "dorny/paths-filter" and
134+
u.getArgument("list-files") = ["csv", "json"] and
135+
u = pred.asExpr() and
136+
o.getStepId() = u.getId() and
137+
o.getFieldName().matches("%_files") and
138+
succ.asExpr() = o
139+
)
140+
}
141+
142+
class DornyPathsFilterTaintStep extends AdditionalTaintStep {
143+
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
144+
dornyPathsFilterTaintStep(node1, node2)
145+
}
146+
}

‎ql/lib/ext/dorny_paths-filter.model.yml

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎ql/test/library-tests/test.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ sources
438438
| amannn/action-semantic-pull-request | * | output.error_message | text | manual |
439439
| cypress-io/github-action | * | env.GH_BRANCH | branch | manual |
440440
| dawidd6/action-download-artifact | * | output.artifacts | artifact | manual |
441-
| dorny/paths-filter | * | output.changes | filename | manual |
442441
| franzdiebold/github-env-vars-action | * | output.CI_PR_DESCRIPTION | text | manual |
443442
| franzdiebold/github-env-vars-action | * | output.CI_PR_TITLE | title | manual |
444443
| googlecloudplatform/magic-modules | * | output.changed-files | filename | manual |

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
name: List files
2+
3+
on:
4+
pull_request_target:
5+
types: [ opened, synchronize, workflow_dispatch]
6+
7+
permissions: {}
8+
jobs:
9+
test:
10+
permissions:
11+
contents: write
12+
pull-requests: write
13+
runs-on: ubuntu-latest
14+
env:
15+
GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
16+
steps:
17+
- name: Check for relevant changes
18+
uses: dorny/paths-filter@v3
19+
id: changed
20+
with:
21+
list-files: json
22+
filters: |
23+
locale:
24+
- '*.xml'
25+
- name: Changed files 1
26+
run: |
27+
echo changed: ${{ steps.changed.outputs.locale_files }}
28+
echo changed: ${{ steps.changed.outputs.changes }}
29+
- name: Check for relevant changes
30+
uses: dorny/paths-filter@v3
31+
id: changed2
32+
with:
33+
list-files: csv
34+
filters: |
35+
locale:
36+
- '*.xml'
37+
- name: Changed files 2
38+
run: |
39+
echo changed:${{ steps.changed2.outputs.locale_files }}
40+
echo changed: ${{ steps.changed2.outputs.changes }}
41+
- name: Check for relevant changes
42+
uses: dorny/paths-filter@v3
43+
id: changed3
44+
with:
45+
list-files: shell
46+
filters: |
47+
locale:
48+
- '*.xml'
49+
- name: Changed files 3
50+
run: |
51+
echo changed:${{ steps.changed3.outputs.locale_files }}
52+
echo changed: ${{ steps.changed3.outputs.changes }}
53+
- name: Check for relevant changes
54+
uses: dorny/paths-filter@v3
55+
id: changed4
56+
with:
57+
list-files: escape
58+
filters: |
59+
locale:
60+
- '*.xml'
61+
- name: Changed files 4
62+
run: |
63+
echo changed:${{ steps.changed4.outputs.locale_files }}
64+
echo changed: ${{ steps.changed4.outputs.changes }}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ edges
5555
| .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value |
5656
| .github/workflows/simple2.yml:22:20:22:64 | steps.source.outputs.all_changed_files | .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] |
5757
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY |
58+
| .github/workflows/test2.yml:17:9:25:6 | Uses Step: changed | .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files |
59+
| .github/workflows/test2.yml:29:9:37:6 | Uses Step: changed2 | .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files |
5860
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] |
5961
| .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] |
6062
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value |
@@ -210,6 +212,10 @@ nodes
210212
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
211213
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
212214
| .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY | semmle.label | env.ISSUE_KEY |
215+
| .github/workflows/test2.yml:17:9:25:6 | Uses Step: changed | semmle.label | Uses Step: changed |
216+
| .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files | semmle.label | steps.changed.outputs.locale_files |
217+
| .github/workflows/test2.yml:29:9:37:6 | Uses Step: changed2 | semmle.label | Uses Step: changed2 |
218+
| .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files | semmle.label | steps.changed2.outputs.locale_files |
213219
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
214220
| .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 |
215221
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ edges
5555
| .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value |
5656
| .github/workflows/simple2.yml:22:20:22:64 | steps.source.outputs.all_changed_files | .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] |
5757
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY |
58+
| .github/workflows/test2.yml:17:9:25:6 | Uses Step: changed | .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files |
59+
| .github/workflows/test2.yml:29:9:37:6 | Uses Step: changed2 | .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files |
5860
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] |
5961
| .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] |
6062
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value |
@@ -210,6 +212,10 @@ nodes
210212
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
211213
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
212214
| .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY | semmle.label | env.ISSUE_KEY |
215+
| .github/workflows/test2.yml:17:9:25:6 | Uses Step: changed | semmle.label | Uses Step: changed |
216+
| .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files | semmle.label | steps.changed.outputs.locale_files |
217+
| .github/workflows/test2.yml:29:9:37:6 | Uses Step: changed2 | semmle.label | Uses Step: changed2 |
218+
| .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files | semmle.label | steps.changed2.outputs.locale_files |
213219
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
214220
| .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 |
215221
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |
@@ -319,6 +325,8 @@ subpaths
319325
| .github/workflows/simple3.yml:22:11:22:37 | toJSON(github.event) | .github/workflows/simple3.yml:22:11:22:37 | toJSON(github.event) | .github/workflows/simple3.yml:22:11:22:37 | toJSON(github.event) | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/simple3.yml:22:11:22:37 | toJSON(github.event) | ${{ toJSON(github.event) }} |
320326
| .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |
321327
| .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY | .github/workflows/test1.yml:22:38:22:75 | github.event.pull_request.title | .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/test1.yml:25:20:25:39 | env.ISSUE_KEY | ${{ env.ISSUE_KEY }} |
328+
| .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files | .github/workflows/test2.yml:17:9:25:6 | Uses Step: changed | .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/test2.yml:27:26:27:66 | steps.changed.outputs.locale_files | ${{ steps.changed.outputs.locale_files }} |
329+
| .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files | .github/workflows/test2.yml:29:9:37:6 | Uses Step: changed2 | .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/test2.yml:39:25:39:66 | steps.changed2.outputs.locale_files | ${{ steps.changed2.outputs.locale_files }} |
322330
| .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] | .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] | ${{needs.job1.outputs['job_output']}} |
323331
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | ${{ github.event.workflow_run.display_title }} |
324332
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | ${{ github.event.workflow_run.head_commit.message }} |

0 commit comments

Comments
 (0)