Skip to content

Commit 6875640

Browse files
author
Alvaro Muñoz
committed
Refactor getXXXExpr methods
1 parent 1c2f19f commit 6875640

9 files changed

Lines changed: 307 additions & 50 deletions

File tree

‎ql/lib/codeql/actions/Ast.qll‎

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -214,36 +214,39 @@ class Strategy extends AstNode instanceof YamlMapping {
214214
/**
215215
* Gets a specific matric expression (YamlMapping) by name.
216216
*/
217-
StringLiteral getMatrixVariable(string name) {
217+
StringLiteral getMatrixVar(string name) {
218218
super.lookup("matrix").(YamlMapping).lookup(name) = result
219219
}
220220

221-
string getAMatrixVariableName() {
222-
this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _)
223-
}
221+
/**
222+
* Gets a specific matric expression (YamlMapping) by name.
223+
*/
224+
StringLiteral getAMatrixVar() { super.lookup("matrix").(YamlMapping).lookup(_) = result }
224225
}
225226

226227
/**
227228
* https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds
228229
*/
229-
class Needs extends AstNode {
230+
class Needs extends AstNode instanceof YamlMappingLikeNode {
230231
Job job;
231232

232233
Needs() { job.(YamlMapping).lookup("needs") = this }
233234

234235
Job getJob() { result = job }
235236

236237
Job getANeededJob() {
237-
if this instanceof YamlString
238-
then
239-
result.getId() = this.(YamlString).getValue() and
240-
result.getLocation().getFile() = job.getLocation().getFile()
241-
else
242-
if this instanceof YamlSequence
243-
then
244-
result.getId() = this.(YamlSequence).getElementNode(_).(YamlString).getValue() and
245-
result.getLocation().getFile() = job.getLocation().getFile()
246-
else none()
238+
result.getId() = super.getNode(_).(YamlString).getValue() and
239+
result.getLocation().getFile() = job.getLocation().getFile()
240+
// if this instanceof YamlString
241+
// then
242+
// result.getId() = this.(YamlString).getValue() and
243+
// result.getLocation().getFile() = job.getLocation().getFile()
244+
// else
245+
// if this instanceof YamlSequence
246+
// then
247+
// result.getId() = this.(YamlSequence).getElementNode(_).(YamlString).getValue() and
248+
// result.getLocation().getFile() = job.getLocation().getFile()
249+
// else none()
247250
}
248251
}
249252

@@ -583,29 +586,30 @@ class StepsExpression extends ContextExpression {
583586
* e.g. `${{ needs.job1.outputs.foo}}`
584587
*/
585588
class NeedsExpression extends ContextExpression {
586-
Job job;
587-
string jobId;
589+
Job neededJob;
590+
string neededJobId;
588591
string fieldName;
589592

590593
NeedsExpression() {
591594
expr.regexpMatch(needsCtxRegex()) and
592-
jobId = expr.regexpCapture(needsCtxRegex(), 1) and
595+
neededJobId = expr.regexpCapture(needsCtxRegex(), 1) and
593596
fieldName = expr.regexpCapture(needsCtxRegex(), 2) and
594-
job.getId() = jobId
597+
neededJob.getId() = neededJobId
595598
}
596599

597-
predicate usesReusableWorkflow() { job.usesReusableWorkflow() }
600+
predicate usesReusableWorkflow() { neededJob.usesReusableWorkflow() }
598601

599602
override string getFieldName() { result = fieldName }
600603

601604
override AstNode getTarget() {
602-
job.getLocation().getFile() = this.getLocation().getFile() and
605+
neededJob.getLocation().getFile() = this.getLocation().getFile() and
606+
this.getJob().getANeededJob() = neededJob and
603607
(
604608
// regular jobs
605-
job.getOutputs() = result
609+
neededJob.getOutputs() = result
606610
or
607611
// reusable workflow calling jobs
608-
job.getUses() = result
612+
neededJob.getUses() = result
609613
)
610614
}
611615
}
@@ -701,12 +705,12 @@ class MatrixExpression extends ContextExpression {
701705

702706
override AstNode getTarget() {
703707
exists(Workflow w |
704-
w.getStrategy().getMatrixVariable(fieldName) = result and
708+
w.getStrategy().getMatrixVar(fieldName) = result and
705709
w.getAChildNode*() = this
706710
)
707711
or
708712
exists(Job j |
709-
j.getStrategy().getMatrixVariable(fieldName) = result and
713+
j.getStrategy().getMatrixVar(fieldName) = result and
710714
j.getAChildNode*() = this
711715
)
712716
}

‎ql/lib/codeql/actions/controlflow/internal/Cfg.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ private class StrategyTree extends StandardPreOrderTree instanceof Strategy {
215215
override ControlFlowTree getChildNode(int i) {
216216
result =
217217
rank[i](AstNode child, Location l |
218-
child = super.getMatrixVariable(_) and l = child.getLocation()
218+
child = super.getAMatrixVar() and l = child.getLocation()
219219
|
220220
child
221221
order by

ql/test/query-tests/Security/CWE-094/.github/workflows/inter-job.yml renamed to ql/test/query-tests/Security/CWE-094/.github/workflows/inter-job0.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
on: push
1+
jn: push
22

33
jobs:
4+
job0:
5+
runs-on: ubuntu-latest
6+
outputs:
7+
job_output: foo
8+
steps:
9+
- run: echo "foo"
10+
411
job1:
512
runs-on: ubuntu-latest
613

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
on: push
2+
3+
jobs:
4+
job0:
5+
runs-on: ubuntu-latest
6+
outputs:
7+
job_output: foo
8+
steps:
9+
- run: echo "foo"
10+
11+
job1:
12+
runs-on: ubuntu-latest
13+
14+
outputs:
15+
job_output: ${{ steps.step.outputs.value }}
16+
17+
steps:
18+
- uses: actions/checkout@v4
19+
with:
20+
fetch-depth: 0
21+
22+
- name: Get changed files
23+
id: source
24+
uses: tj-actions/changed-files@v40
25+
26+
- name: Remove foo from changed files
27+
id: step
28+
uses: mad9000/actions-find-and-replace-string@3
29+
with:
30+
source: ${{ steps.source.outputs.all_changed_files }}
31+
find: 'foo'
32+
replace: ''
33+
34+
job2:
35+
runs-on: ubuntu-latest
36+
37+
if: ${{ always() }}
38+
39+
needs: [job0, job1]
40+
41+
steps:
42+
- id: sink
43+
run: echo ${{needs.job1.outputs.job_output}}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
on: push
2+
3+
jobs:
4+
job0:
5+
runs-on: ubuntu-latest
6+
outputs:
7+
job_output: foo
8+
steps:
9+
- run: echo "foo"
10+
11+
job1:
12+
runs-on: ubuntu-latest
13+
14+
outputs:
15+
job_output: ${{ steps.step.outputs.value }}
16+
17+
steps:
18+
- uses: actions/checkout@v4
19+
with:
20+
fetch-depth: 0
21+
22+
- name: Get changed files
23+
id: source
24+
uses: tj-actions/changed-files@v40
25+
26+
- name: Remove foo from changed files
27+
id: step
28+
uses: mad9000/actions-find-and-replace-string@3
29+
with:
30+
source: ${{ steps.source.outputs.all_changed_files }}
31+
find: 'foo'
32+
replace: ''
33+
34+
job2:
35+
runs-on: ubuntu-latest
36+
37+
if: ${{ always() }}
38+
39+
needs:
40+
- job0
41+
- job1
42+
43+
steps:
44+
- id: sink
45+
run: echo ${{needs.job1.outputs.job_output}}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
jn: push
2+
3+
jobs:
4+
job0:
5+
runs-on: ubuntu-latest
6+
outputs:
7+
job_output: foo
8+
steps:
9+
- run: echo "foo"
10+
11+
job1:
12+
runs-on: ubuntu-latest
13+
14+
outputs:
15+
job_output: ${{ steps.step.outputs.value }}
16+
17+
steps:
18+
- uses: actions/checkout@v4
19+
with:
20+
fetch-depth: 0
21+
22+
- name: Get changed files
23+
id: source
24+
uses: tj-actions/changed-files@v40
25+
26+
- name: Remove foo from changed files
27+
id: step
28+
uses: mad9000/actions-find-and-replace-string@3
29+
with:
30+
source: ${{ steps.source.outputs.all_changed_files }}
31+
find: 'foo'
32+
replace: ''
33+
34+
job2:
35+
runs-on: ubuntu-latest
36+
37+
if: ${{ always() }}
38+
39+
needs:
40+
- job1
41+
42+
steps:
43+
- id: sink
44+
run: echo ${{needs.job1.outputs.job_output}}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
jn: push
2+
3+
jobs:
4+
job0:
5+
runs-on: ubuntu-latest
6+
outputs:
7+
job_output: foo
8+
steps:
9+
- run: echo "foo"
10+
11+
job1:
12+
runs-on: ubuntu-latest
13+
14+
outputs:
15+
job_output: ${{ steps.step.outputs.value }}
16+
17+
steps:
18+
- uses: actions/checkout@v4
19+
with:
20+
fetch-depth: 0
21+
22+
- name: Get changed files
23+
id: source
24+
uses: tj-actions/changed-files@v40
25+
26+
- name: Remove foo from changed files
27+
id: step
28+
uses: mad9000/actions-find-and-replace-string@3
29+
with:
30+
source: ${{ steps.source.outputs.all_changed_files }}
31+
find: 'foo'
32+
replace: ''
33+
34+
job2:
35+
runs-on: ubuntu-latest
36+
37+
if: ${{ always() }}
38+
39+
needs:
40+
- job0
41+
42+
steps:
43+
- id: sink
44+
# Should not be reported since job1 is not needed
45+
run: echo ${{needs.job1.outputs.job_output}}

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

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,26 @@ edges
1515
| .github/workflows/image_link_generator.yml:25:24:25:67 | ${{ ste ... _url }} | .github/workflows/image_link_generator.yml:22:9:28:6 | Run Step: curl [redirected_url] |
1616
| .github/workflows/image_link_generator.yml:28:9:35:6 | Run Step: trim-url [trimmed_url] | .github/workflows/image_link_generator.yml:36:14:37:126 | \| |
1717
| .github/workflows/image_link_generator.yml:31:27:31:66 | ${{ ste ... _url }} | .github/workflows/image_link_generator.yml:28:9:35:6 | Run Step: trim-url [trimmed_url] |
18-
| .github/workflows/inter-job.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/inter-job.yml:36:14:36:52 | echo ${ ... utput}} |
19-
| .github/workflows/inter-job.yml:8:19:8:49 | ${{ ste ... alue }} | .github/workflows/inter-job.yml:8:7:10:4 | Job outputs node [job_output] |
20-
| .github/workflows/inter-job.yml:15:9:19:6 | Uses Step: source | .github/workflows/inter-job.yml:23:19:23:63 | ${{ ste ... iles }} |
21-
| .github/workflows/inter-job.yml:19:9:27:2 | Uses Step: step [value] | .github/workflows/inter-job.yml:8:19:8:49 | ${{ ste ... alue }} |
22-
| .github/workflows/inter-job.yml:23:19:23:63 | ${{ ste ... iles }} | .github/workflows/inter-job.yml:19:9:27:2 | Uses Step: step [value] |
18+
| .github/workflows/inter-job0.yml:15:7:17:4 | Job outputs node [job_output] | .github/workflows/inter-job0.yml:43:14:43:52 | echo ${ ... utput}} |
19+
| .github/workflows/inter-job0.yml:15:19:15:49 | ${{ ste ... alue }} | .github/workflows/inter-job0.yml:15:7:17:4 | Job outputs node [job_output] |
20+
| .github/workflows/inter-job0.yml:22:9:26:6 | Uses Step: source | .github/workflows/inter-job0.yml:30:19:30:63 | ${{ ste ... iles }} |
21+
| .github/workflows/inter-job0.yml:26:9:34:2 | Uses Step: step [value] | .github/workflows/inter-job0.yml:15:19:15:49 | ${{ ste ... alue }} |
22+
| .github/workflows/inter-job0.yml:30:19:30:63 | ${{ ste ... iles }} | .github/workflows/inter-job0.yml:26:9:34:2 | Uses Step: step [value] |
23+
| .github/workflows/inter-job1.yml:15:7:17:4 | Job outputs node [job_output] | .github/workflows/inter-job1.yml:43:14:43:52 | echo ${ ... utput}} |
24+
| .github/workflows/inter-job1.yml:15:19:15:49 | ${{ ste ... alue }} | .github/workflows/inter-job1.yml:15:7:17:4 | Job outputs node [job_output] |
25+
| .github/workflows/inter-job1.yml:22:9:26:6 | Uses Step: source | .github/workflows/inter-job1.yml:30:19:30:63 | ${{ ste ... iles }} |
26+
| .github/workflows/inter-job1.yml:26:9:34:2 | Uses Step: step [value] | .github/workflows/inter-job1.yml:15:19:15:49 | ${{ ste ... alue }} |
27+
| .github/workflows/inter-job1.yml:30:19:30:63 | ${{ ste ... iles }} | .github/workflows/inter-job1.yml:26:9:34:2 | Uses Step: step [value] |
28+
| .github/workflows/inter-job2.yml:15:7:17:4 | Job outputs node [job_output] | .github/workflows/inter-job2.yml:45:14:45:52 | echo ${ ... utput}} |
29+
| .github/workflows/inter-job2.yml:15:19:15:49 | ${{ ste ... alue }} | .github/workflows/inter-job2.yml:15:7:17:4 | Job outputs node [job_output] |
30+
| .github/workflows/inter-job2.yml:22:9:26:6 | Uses Step: source | .github/workflows/inter-job2.yml:30:19:30:63 | ${{ ste ... iles }} |
31+
| .github/workflows/inter-job2.yml:26:9:34:2 | Uses Step: step [value] | .github/workflows/inter-job2.yml:15:19:15:49 | ${{ ste ... alue }} |
32+
| .github/workflows/inter-job2.yml:30:19:30:63 | ${{ ste ... iles }} | .github/workflows/inter-job2.yml:26:9:34:2 | Uses Step: step [value] |
33+
| .github/workflows/inter-job4.yml:15:7:17:4 | Job outputs node [job_output] | .github/workflows/inter-job4.yml:44:14:44:52 | echo ${ ... utput}} |
34+
| .github/workflows/inter-job4.yml:15:19:15:49 | ${{ ste ... alue }} | .github/workflows/inter-job4.yml:15:7:17:4 | Job outputs node [job_output] |
35+
| .github/workflows/inter-job4.yml:22:9:26:6 | Uses Step: source | .github/workflows/inter-job4.yml:30:19:30:63 | ${{ ste ... iles }} |
36+
| .github/workflows/inter-job4.yml:26:9:34:2 | Uses Step: step [value] | .github/workflows/inter-job4.yml:15:19:15:49 | ${{ ste ... alue }} |
37+
| .github/workflows/inter-job4.yml:30:19:30:63 | ${{ ste ... iles }} | .github/workflows/inter-job4.yml:26:9:34:2 | Uses Step: step [value] |
2338
| .github/workflows/issues.yaml:4:15:4:45 | ${{ git ... itle }} | .github/workflows/issues.yaml:15:12:15:39 | echo '$ ... env }}' |
2439
| .github/workflows/issues.yaml:10:16:10:46 | ${{ git ... itle }} | .github/workflows/issues.yaml:17:12:17:36 | echo '$ ... env }}' |
2540
| .github/workflows/issues.yaml:20:19:20:49 | ${{ git ... itle }} | .github/workflows/issues.yaml:18:12:18:37 | echo '$ ... env }}' |
@@ -75,12 +90,30 @@ nodes
7590
| .github/workflows/image_link_generator.yml:28:9:35:6 | Run Step: trim-url [trimmed_url] | semmle.label | Run Step: trim-url [trimmed_url] |
7691
| .github/workflows/image_link_generator.yml:31:27:31:66 | ${{ ste ... _url }} | semmle.label | ${{ ste ... _url }} |
7792
| .github/workflows/image_link_generator.yml:36:14:37:126 | \| | semmle.label | \| |
78-
| .github/workflows/inter-job.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
79-
| .github/workflows/inter-job.yml:8:19:8:49 | ${{ ste ... alue }} | semmle.label | ${{ ste ... alue }} |
80-
| .github/workflows/inter-job.yml:15:9:19:6 | Uses Step: source | semmle.label | Uses Step: source |
81-
| .github/workflows/inter-job.yml:19:9:27:2 | Uses Step: step [value] | semmle.label | Uses Step: step [value] |
82-
| .github/workflows/inter-job.yml:23:19:23:63 | ${{ ste ... iles }} | semmle.label | ${{ ste ... iles }} |
83-
| .github/workflows/inter-job.yml:36:14:36:52 | echo ${ ... utput}} | semmle.label | echo ${ ... utput}} |
93+
| .github/workflows/inter-job0.yml:15:7:17:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
94+
| .github/workflows/inter-job0.yml:15:19:15:49 | ${{ ste ... alue }} | semmle.label | ${{ ste ... alue }} |
95+
| .github/workflows/inter-job0.yml:22:9:26:6 | Uses Step: source | semmle.label | Uses Step: source |
96+
| .github/workflows/inter-job0.yml:26:9:34:2 | Uses Step: step [value] | semmle.label | Uses Step: step [value] |
97+
| .github/workflows/inter-job0.yml:30:19:30:63 | ${{ ste ... iles }} | semmle.label | ${{ ste ... iles }} |
98+
| .github/workflows/inter-job0.yml:43:14:43:52 | echo ${ ... utput}} | semmle.label | echo ${ ... utput}} |
99+
| .github/workflows/inter-job1.yml:15:7:17:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
100+
| .github/workflows/inter-job1.yml:15:19:15:49 | ${{ ste ... alue }} | semmle.label | ${{ ste ... alue }} |
101+
| .github/workflows/inter-job1.yml:22:9:26:6 | Uses Step: source | semmle.label | Uses Step: source |
102+
| .github/workflows/inter-job1.yml:26:9:34:2 | Uses Step: step [value] | semmle.label | Uses Step: step [value] |
103+
| .github/workflows/inter-job1.yml:30:19:30:63 | ${{ ste ... iles }} | semmle.label | ${{ ste ... iles }} |
104+
| .github/workflows/inter-job1.yml:43:14:43:52 | echo ${ ... utput}} | semmle.label | echo ${ ... utput}} |
105+
| .github/workflows/inter-job2.yml:15:7:17:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
106+
| .github/workflows/inter-job2.yml:15:19:15:49 | ${{ ste ... alue }} | semmle.label | ${{ ste ... alue }} |
107+
| .github/workflows/inter-job2.yml:22:9:26:6 | Uses Step: source | semmle.label | Uses Step: source |
108+
| .github/workflows/inter-job2.yml:26:9:34:2 | Uses Step: step [value] | semmle.label | Uses Step: step [value] |
109+
| .github/workflows/inter-job2.yml:30:19:30:63 | ${{ ste ... iles }} | semmle.label | ${{ ste ... iles }} |
110+
| .github/workflows/inter-job2.yml:45:14:45:52 | echo ${ ... utput}} | semmle.label | echo ${ ... utput}} |
111+
| .github/workflows/inter-job4.yml:15:7:17:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
112+
| .github/workflows/inter-job4.yml:15:19:15:49 | ${{ ste ... alue }} | semmle.label | ${{ ste ... alue }} |
113+
| .github/workflows/inter-job4.yml:22:9:26:6 | Uses Step: source | semmle.label | Uses Step: source |
114+
| .github/workflows/inter-job4.yml:26:9:34:2 | Uses Step: step [value] | semmle.label | Uses Step: step [value] |
115+
| .github/workflows/inter-job4.yml:30:19:30:63 | ${{ ste ... iles }} | semmle.label | ${{ ste ... iles }} |
116+
| .github/workflows/inter-job4.yml:44:14:44:52 | echo ${ ... utput}} | semmle.label | echo ${ ... utput}} |
84117
| .github/workflows/issues.yaml:4:15:4:45 | ${{ git ... itle }} | semmle.label | ${{ git ... itle }} |
85118
| .github/workflows/issues.yaml:10:16:10:46 | ${{ git ... itle }} | semmle.label | ${{ git ... itle }} |
86119
| .github/workflows/issues.yaml:13:12:13:49 | echo '$ ... tle }}' | semmle.label | echo '$ ... tle }}' |

0 commit comments

Comments
 (0)