Skip to content

Commit 872b1f8

Browse files
author
Alvaro Muñoz
committed
More regexp improvements
1 parent 0e50204 commit 872b1f8

File tree

6 files changed

+20
-18
lines changed

6 files changed

+20
-18
lines changed

ql/lib/codeql/actions/Ast.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ module Utils {
55
bindingset[expr]
66
string normalizeExpr(string expr) {
77
result =
8-
expr.replaceAll("['", ".")
9-
.replaceAll("']", "")
10-
.replaceAll("[\"", ".")
11-
.replaceAll("\"]", "")
8+
//[A-Za-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]
9+
expr.regexpReplaceAll("\\['([a-zA-Z0-9_\\*\\-]+)'\\]", ".$1")
10+
.regexpReplaceAll("\\[\"([a-zA-Z0-9_\\*\\-]+)\"\\]", ".$1")
1211
.regexpReplaceAll("\\s*\\.\\s*", ".")
1312
}
1413
}
@@ -45,6 +44,8 @@ class Expression extends AstNode instanceof ExpressionImpl {
4544
string getExpression() { result = expression }
4645

4746
string getRawExpression() { result = rawExpression }
47+
48+
string getNormalizedExpression() { result = Utils::normalizeExpr(expression) }
4849
}
4950

5051
/** A common class for `env` in workflow, job or step. */

ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ string getASimpleReferenceExpression(YamlString s, int offset) {
3030
// not just the last (greedy match) or first (reluctant match).
3131
result =
3232
s.getValue()
33-
.regexpFind("\\$\\{\\{\\s*[A-Za-z0-9_\\[\\]\\*\\(\\)\\.\\-]+\\s*\\}\\}", _, offset)
34-
.regexpCapture("(\\$\\{\\{\\s*[A-Za-z0-9_\\[\\]\\*\\((\\)\\.\\-]+\\s*\\}\\})", 1)
33+
.regexpFind("\\$\\{\\{\\s*[A-Za-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\s*\\}\\}", _, offset)
34+
.regexpCapture("(\\$\\{\\{\\s*[A-Za-z0-9'\"_\\[\\]\\*\\((\\)\\.\\-]+\\s*\\}\\})", 1)
3535
}
3636

3737
private newtype TAstNode =

ql/src/Debug/partial.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
import actions
11+
import codeql.actions.DataFlow
1112
import codeql.actions.TaintTracking
1213
import codeql.actions.dataflow.FlowSources
1314
import PartialFlow::PartialPathGraph

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ jobs:
1212
- id: step0
1313
uses: mad9000/actions-find-and-replace-string@3
1414
with:
15-
source: ${{ github.event.head_commit.message }}
15+
source: ${{ github.event['head_commit']['message'] }}
1616
find: 'foo'
1717
replace: ''
1818
- id: step1
@@ -34,4 +34,4 @@ jobs:
3434
needs: job1
3535

3636
steps:
37-
- run: echo ${{needs.job1.outputs.job_output}}
37+
- run: echo ${{needs.job1.outputs['job_output']}}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ edges
4444
| .github/workflows/simple2.yml:14:9:18:6 | Uses Step: source | .github/workflows/simple2.yml:22:20:22:64 | steps.source.outputs.all_changed_files |
4545
| .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value |
4646
| .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] |
47-
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output |
47+
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] |
4848
| .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] |
4949
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value |
50-
| .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] |
50+
| .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] |
5151
| .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] | .github/workflows/test.yml:26:19:26:46 | steps.step1.outputs.MSG |
5252
| .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] |
5353
| .github/workflows/test.yml:24:9:29:2 | Run Step: step2 [test] | .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test |
@@ -172,12 +172,12 @@ nodes
172172
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
173173
| .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test | semmle.label | steps.step2.outputs.test |
174174
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |
175-
| .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | semmle.label | github.event.head_commit.message |
175+
| .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | semmle.label | github.event['head_commit']['message'] |
176176
| .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] | semmle.label | Run Step: step1 [MSG] |
177177
| .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | semmle.label | steps.step0.outputs.value |
178178
| .github/workflows/test.yml:24:9:29:2 | Run Step: step2 [test] | semmle.label | Run Step: step2 [test] |
179179
| .github/workflows/test.yml:26:19:26:46 | steps.step1.outputs.MSG | semmle.label | steps.step1.outputs.MSG |
180-
| .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | semmle.label | needs.job1.outputs.job_output |
180+
| .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | semmle.label | needs.job1.outputs['job_output'] |
181181
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | semmle.label | github.event.workflow_run.display_title |
182182
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | semmle.label | github.event.workflow_run.head_commit.message |
183183
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | semmle.label | github.event.workflow_run.head_commit.author.email |
@@ -254,7 +254,7 @@ subpaths
254254
| .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | github.event.commits[11].committer.name |
255255
| .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | .github/workflows/simple1.yml:11:20:11:58 | github.event.head_commit.message | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | steps.summary.outputs.value |
256256
| .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | .github/workflows/simple2.yml:14:9:18:6 | Uses Step: source | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | steps.step.outputs.value |
257-
| .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | needs.job1.outputs.job_output |
257+
| .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | needs.job1.outputs['job_output'] |
258258
| .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 expression 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 |
259259
| .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 expression 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 |
260260
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | github.event.workflow_run.head_commit.author.email |

ql/test/query-tests/Security/CWE-094/ExpressionInjection.expected

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ edges
4444
| .github/workflows/simple2.yml:14:9:18:6 | Uses Step: source | .github/workflows/simple2.yml:22:20:22:64 | steps.source.outputs.all_changed_files |
4545
| .github/workflows/simple2.yml:18:9:26:6 | Uses Step: step [value] | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value |
4646
| .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] |
47-
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output |
47+
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] |
4848
| .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] |
4949
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value |
50-
| .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] |
50+
| .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] |
5151
| .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] | .github/workflows/test.yml:26:19:26:46 | steps.step1.outputs.MSG |
5252
| .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] |
5353
| .github/workflows/test.yml:24:9:29:2 | Run Step: step2 [test] | .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test |
@@ -172,12 +172,12 @@ nodes
172172
| .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
173173
| .github/workflows/test.yml:8:20:8:50 | steps.step2.outputs.test | semmle.label | steps.step2.outputs.test |
174174
| .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |
175-
| .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | semmle.label | github.event.head_commit.message |
175+
| .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | semmle.label | github.event['head_commit']['message'] |
176176
| .github/workflows/test.yml:18:9:24:6 | Run Step: step1 [MSG] | semmle.label | Run Step: step1 [MSG] |
177177
| .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | semmle.label | steps.step0.outputs.value |
178178
| .github/workflows/test.yml:24:9:29:2 | Run Step: step2 [test] | semmle.label | Run Step: step2 [test] |
179179
| .github/workflows/test.yml:26:19:26:46 | steps.step1.outputs.MSG | semmle.label | steps.step1.outputs.MSG |
180-
| .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | semmle.label | needs.job1.outputs.job_output |
180+
| .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | semmle.label | needs.job1.outputs['job_output'] |
181181
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | semmle.label | github.event.workflow_run.display_title |
182182
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | semmle.label | github.event.workflow_run.head_commit.message |
183183
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | semmle.label | github.event.workflow_run.head_commit.author.email |
@@ -261,7 +261,7 @@ subpaths
261261
| .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/push.yml:16:19:16:64 | github.event.commits[11].committer.name | ${{ github.event.commits[11].committer.name }} |
262262
| .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | .github/workflows/simple1.yml:11:20:11:58 | github.event.head_commit.message | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/simple1.yml:16:18:16:49 | steps.summary.outputs.value | ${{steps.summary.outputs.value}} |
263263
| .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | .github/workflows/simple2.yml:14:9:18:6 | Uses Step: source | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/simple2.yml:29:24:29:54 | steps.step.outputs.value | ${{ steps.step.outputs.value }} |
264-
| .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | .github/workflows/test.yml:15:20:15:58 | github.event.head_commit.message | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:37:20:37:53 | needs.job1.outputs.job_output | ${{needs.job1.outputs.job_output}} |
264+
| .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | .github/workflows/test.yml:15:20:15:64 | github.event['head_commit']['message'] | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:37:20:37:56 | needs.job1.outputs['job_output'] | ${{needs.job1.outputs['job_output']}} |
265265
| .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 expression 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 }} |
266266
| .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 expression 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 }} |
267267
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | Potential expression injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | ${{ github.event.workflow_run.head_commit.author.email }} |

0 commit comments

Comments
 (0)