From 4b01cd5be45dc8ebb161e7e55ec4e4ef7b8172c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= Date: Tue, 2 Jul 2024 23:51:19 +0200 Subject: [PATCH] Support flow through fromJson --- ql/lib/codeql/actions/ast/internal/Ast.qll | 101 +++++++++++++++--- .../CWE-094/.github/workflows/test9.yml | 27 +++++ .../CWE-094/CodeInjectionCritical.expected | 17 +++ .../CWE-094/CodeInjectionMedium.expected | 13 +++ 4 files changed, 141 insertions(+), 17 deletions(-) create mode 100644 ql/test/query-tests/Security/CWE-094/.github/workflows/test9.yml diff --git a/ql/lib/codeql/actions/ast/internal/Ast.qll b/ql/lib/codeql/actions/ast/internal/Ast.qll index 9d2a5b38206..c6569367c10 100644 --- a/ql/lib/codeql/actions/ast/internal/Ast.qll +++ b/ql/lib/codeql/actions/ast/internal/Ast.qll @@ -1194,12 +1194,25 @@ string getASimpleReferenceExpression(string s, int offset) { .regexpCapture("([A-Za-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)", 1) } +bindingset[s] +string getAJsonReferenceExpression(string s, int offset) { + // We use `regexpFind` to obtain *all* matches of `${{...}}`, + // not just the last (greedy match) or first (reluctant match). + result = + s.trim() + .regexpFind("(?i)fromjson\\([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\).*", _, offset) + .regexpCapture("(?i)fromjson\\(([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)\\).*", 1) +} + /** * A ${{}} expression accessing a context variable such as steps, needs, jobs, env, inputs, or matrix. * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability */ abstract class SimpleReferenceExpressionImpl extends ExpressionImpl { - SimpleReferenceExpressionImpl() { exists(getASimpleReferenceExpression(expression, _)) } + SimpleReferenceExpressionImpl() { + exists(getASimpleReferenceExpression(expression, _)) or + exists(getAJsonReferenceExpression(expression, _)) + } abstract string getFieldName(); @@ -1236,8 +1249,17 @@ class SecretsExpressionImpl extends SimpleReferenceExpressionImpl { string fieldName; SecretsExpressionImpl() { - normalizeExpr(expression).regexpMatch(secretsCtxRegex()) and - fieldName = normalizeExpr(expression).regexpCapture(secretsCtxRegex(), 1) + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(secretsCtxRegex()) and + fieldName = expr.regexpCapture(secretsCtxRegex(), 1) + ) } override string getFieldName() { result = fieldName } @@ -1255,9 +1277,18 @@ class StepsExpressionImpl extends SimpleReferenceExpressionImpl { string fieldName; StepsExpressionImpl() { - normalizeExpr(expression).regexpMatch(stepsCtxRegex()) and - stepId = normalizeExpr(expression).regexpCapture(stepsCtxRegex(), 1) and - fieldName = normalizeExpr(expression).regexpCapture(stepsCtxRegex(), 2) + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(stepsCtxRegex()) and + stepId = expr.regexpCapture(stepsCtxRegex(), 1) and + fieldName = expr.regexpCapture(stepsCtxRegex(), 2) + ) } override string getFieldName() { result = fieldName } @@ -1287,10 +1318,19 @@ class NeedsExpressionImpl extends SimpleReferenceExpressionImpl { string fieldName; NeedsExpressionImpl() { - normalizeExpr(expression).regexpMatch(needsCtxRegex()) and - fieldName = normalizeExpr(expression).regexpCapture(needsCtxRegex(), 2) and - neededJob.getId() = normalizeExpr(expression).regexpCapture(needsCtxRegex(), 1) and - neededJob.getLocation().getFile() = this.getLocation().getFile() + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(needsCtxRegex()) and + fieldName = expr.regexpCapture(needsCtxRegex(), 2) and + neededJob.getId() = expr.regexpCapture(needsCtxRegex(), 1) and + neededJob.getLocation().getFile() = this.getLocation().getFile() + ) } override string getFieldName() { result = fieldName } @@ -1320,9 +1360,18 @@ class JobsExpressionImpl extends SimpleReferenceExpressionImpl { string fieldName; JobsExpressionImpl() { - normalizeExpr(expression).regexpMatch(jobsCtxRegex()) and - jobId = normalizeExpr(expression).regexpCapture(jobsCtxRegex(), 1) and - fieldName = normalizeExpr(expression).regexpCapture(jobsCtxRegex(), 2) + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(jobsCtxRegex()) and + jobId = expr.regexpCapture(jobsCtxRegex(), 1) and + fieldName = expr.regexpCapture(jobsCtxRegex(), 2) + ) } override string getFieldName() { result = fieldName } @@ -1370,8 +1419,17 @@ class EnvExpressionImpl extends SimpleReferenceExpressionImpl { string fieldName; EnvExpressionImpl() { - normalizeExpr(expression).regexpMatch(envCtxRegex()) and - fieldName = normalizeExpr(expression).regexpCapture(envCtxRegex(), 1) + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(envCtxRegex()) and + fieldName = expr.regexpCapture(envCtxRegex(), 1) + ) } override string getFieldName() { result = fieldName } @@ -1396,8 +1454,17 @@ class MatrixExpressionImpl extends SimpleReferenceExpressionImpl { string fieldAccess; MatrixExpressionImpl() { - normalizeExpr(expression).regexpMatch(matrixCtxRegex()) and - fieldAccess = normalizeExpr(expression).regexpCapture(matrixCtxRegex(), 1) + exists(string expr | + ( + exists(getAJsonReferenceExpression(expression, _)) and + expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1) + or + exists(getASimpleReferenceExpression(expression, _)) and + expr = normalizeExpr(expression) + ) and + expr.regexpMatch(matrixCtxRegex()) and + fieldAccess = expr.regexpCapture(matrixCtxRegex(), 1) + ) } override string getFieldName() { result = fieldAccess } diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/test9.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/test9.yml new file mode 100644 index 00000000000..6ed7db83cb2 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-094/.github/workflows/test9.yml @@ -0,0 +1,27 @@ +name: Test + +on: + issue_comment: + +jobs: + parse-issue: + runs-on: ubuntu-latest + outputs: + payload: ${{ steps.issue_body_parser_request.outputs.payload }} + steps: + - name: Get JSON Data out of Issue Request + uses: peter-murray/issue-body-parser-action@v2 + id: issue_body_parser_request + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + issue_id: ${{ github.event.issue.number }} + payload_marker: request + fail_on_missing: false + - run: echo ${{ steps.issue_body_parser_request.outputs.payload }} + approve-or-deny-request: + runs-on: ubuntu-latest + needs: parse-issue + steps: + - run: echo ${{ needs.parse-issue.outputs.payload }} + - run: echo ${{ fromJson(needs.parse-issue.outputs.payload) }} + - run: echo ${{ fromJson(needs.parse-issue.outputs.payload).version }} diff --git a/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected b/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected index 1b98263c16e..ff378f93af6 100644 --- a/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected +++ b/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected @@ -67,6 +67,12 @@ edges | .github/workflows/test3.yml:13:9:21:2 | Uses Step: issue_body_parser_request | .github/workflows/test3.yml:11:17:11:70 | steps.issue_body_parser_request.outputs.payload | provenance | | | .github/workflows/test7.yml:9:9:13:6 | Uses Step: comment-branch | .github/workflows/test7.yml:18:37:18:80 | steps.comment-branch.outputs.head_ref | provenance | | | .github/workflows/test7.yml:13:9:17:6 | Uses Step: refs | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | provenance | | +| .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | provenance | | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | provenance | | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | provenance | | | .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'] | provenance | | | .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] | provenance | | | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | provenance | | @@ -251,6 +257,13 @@ nodes | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | semmle.label | steps.refs.outputs.head_ref | | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | semmle.label | github.event.pull_request.head.ref | | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | semmle.label | github.event.pull_request.head.ref | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | semmle.label | Job outputs node [payload] | +| .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | semmle.label | steps.issue_body_parser_request.outputs.payload | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | semmle.label | Uses Step: issue_body_parser_request | +| .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | semmle.label | steps.issue_body_parser_request.outputs.payload | +| .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | semmle.label | needs.parse-issue.outputs.payload | +| .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | semmle.label | fromJson(needs.parse-issue.outputs.payload) | +| .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | semmle.label | fromJson(needs.parse-issue.outputs.payload).version | | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] | | .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 | | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] | @@ -352,6 +365,10 @@ subpaths | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | .github/workflows/test7.yml:13:9:17:6 | Uses Step: refs | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | ${{ steps.refs.outputs.head_ref }} | | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | ${{ github.event.pull_request.head.ref }} | | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | ${{ github.event.pull_request.head.ref }} | +| .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | ${{ steps.issue_body_parser_request.outputs.payload }} | +| .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | ${{ needs.parse-issue.outputs.payload }} | +| .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | ${{ fromJson(needs.parse-issue.outputs.payload) }} | +| .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | ${{ fromJson(needs.parse-issue.outputs.payload).version }} | | .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 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 }} | | .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 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 }} | | .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 code 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 }} | diff --git a/ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected b/ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected index 35887c3b370..19b72ad6b5c 100644 --- a/ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected +++ b/ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected @@ -67,6 +67,12 @@ edges | .github/workflows/test3.yml:13:9:21:2 | Uses Step: issue_body_parser_request | .github/workflows/test3.yml:11:17:11:70 | steps.issue_body_parser_request.outputs.payload | provenance | | | .github/workflows/test7.yml:9:9:13:6 | Uses Step: comment-branch | .github/workflows/test7.yml:18:37:18:80 | steps.comment-branch.outputs.head_ref | provenance | | | .github/workflows/test7.yml:13:9:17:6 | Uses Step: refs | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | provenance | | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | provenance | | +| .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | provenance | | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | provenance | | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | provenance | | | .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'] | provenance | | | .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] | provenance | | | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | .github/workflows/test.yml:20:18:20:48 | steps.step0.outputs.value | provenance | | @@ -251,6 +257,13 @@ nodes | .github/workflows/test7.yml:20:37:20:70 | steps.refs.outputs.head_ref | semmle.label | steps.refs.outputs.head_ref | | .github/workflows/test8.yml:24:76:24:116 | github.event.pull_request.head.ref | semmle.label | github.event.pull_request.head.ref | | .github/workflows/test8.yml:30:76:30:116 | github.event.pull_request.head.ref | semmle.label | github.event.pull_request.head.ref | +| .github/workflows/test9.yml:10:7:11:4 | Job outputs node [payload] | semmle.label | Job outputs node [payload] | +| .github/workflows/test9.yml:10:17:10:70 | steps.issue_body_parser_request.outputs.payload | semmle.label | steps.issue_body_parser_request.outputs.payload | +| .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | semmle.label | Uses Step: issue_body_parser_request | +| .github/workflows/test9.yml:20:20:20:73 | steps.issue_body_parser_request.outputs.payload | semmle.label | steps.issue_body_parser_request.outputs.payload | +| .github/workflows/test9.yml:25:18:25:57 | needs.parse-issue.outputs.payload | semmle.label | needs.parse-issue.outputs.payload | +| .github/workflows/test9.yml:26:18:26:67 | fromJson(needs.parse-issue.outputs.payload) | semmle.label | fromJson(needs.parse-issue.outputs.payload) | +| .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | semmle.label | fromJson(needs.parse-issue.outputs.payload).version | | .github/workflows/test.yml:8:7:10:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] | | .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 | | .github/workflows/test.yml:12:9:18:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |