Skip to content

Commit 8323819

Browse files
author
Alvaro Muñoz
committed
New sources for octokit/request-action
1 parent a1047d1 commit 8323819

5 files changed

Lines changed: 158 additions & 1 deletion

File tree

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,24 @@ class Xt0rtedSlashCommandSource extends RemoteFlowSource {
295295

296296
override string getSourceType() { result = "text" }
297297
}
298+
299+
class OctokitRequestActionSource extends RemoteFlowSource {
300+
OctokitRequestActionSource() {
301+
exists(UsesStep u, string route |
302+
u.getCallee() = "octokit/request-action" and
303+
route = u.getArgument("route").trim() and
304+
route.indexOf("GET") = 0 and
305+
(
306+
route.matches("%/commits%") or
307+
route.matches("%/comments%") or
308+
route.matches("%/pulls%") or
309+
route.matches("%/issues%") or
310+
route.matches("%/users%") or
311+
route.matches("%github.event.issue.pull_request.url%")
312+
) and
313+
this.asExpr() = u
314+
)
315+
}
316+
317+
override string getSourceType() { result = "text" }
318+
}

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,45 @@ predicate xt0rtedSlashCommandActionTaintStep(DataFlow::Node pred, DataFlow::Node
9191
)
9292
}
9393

94+
/**
95+
* A read of user-controlled field of the octokit/request-action action.
96+
*/
97+
predicate octokitRequestActionTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
98+
exists(StepsExpression o |
99+
pred instanceof OctokitRequestActionSource and
100+
o.getTarget() = pred.asExpr() and
101+
o.getStepId() = pred.asExpr().(UsesStep).getId() and
102+
succ.asExpr() = o and
103+
(
104+
not o instanceof JsonReferenceExpression and
105+
o.getFieldName() = "data"
106+
or
107+
o instanceof JsonReferenceExpression and
108+
o.(JsonReferenceExpression).getInnerExpression().matches("%.data") and
109+
o.(JsonReferenceExpression)
110+
.getAccessPath()
111+
.matches([
112+
"%.title",
113+
"%.user.login",
114+
"%.body",
115+
"%.head.ref",
116+
"%.head.repo.full_name",
117+
"%.commit.author.email",
118+
"%.commit.commiter.email",
119+
"%.commit.message",
120+
"%.email",
121+
"%.name",
122+
])
123+
)
124+
)
125+
}
126+
94127
class TaintSteps extends AdditionalTaintStep {
95128
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
96129
dornyPathsFilterTaintStep(node1, node2) or
97130
tjActionsChangedFilesTaintStep(node1, node2) or
98131
tjActionsVerifyChangedFilesTaintStep(node1, node2) or
99-
xt0rtedSlashCommandActionTaintStep(node1, node2)
132+
xt0rtedSlashCommandActionTaintStep(node1, node2) or
133+
octokitRequestActionTaintStep(node1, node2)
100134
}
101135
}

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
name: Test
2+
3+
on:
4+
issue_comment:
5+
6+
permissions:
7+
contents: read
8+
pull-requests: write
9+
10+
jobs:
11+
setup:
12+
runs-on: ubuntu-latest
13+
steps:
14+
- name: Get PR details
15+
id: get-pr
16+
if: github.event_name == 'issue_comment'
17+
uses: octokit/request-action@v2.x
18+
with:
19+
route: GET /repos/${{ github.repository }}/pulls/${{ github.event.issue.number }}
20+
env:
21+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
22+
- name: Set PR source branch as env variable
23+
if: github.event_name == 'issue_comment'
24+
run: |
25+
PR_SOURCE_BRANCH=$(echo '${{ steps.get-pr.outputs.data }}' | jq -r '.head.ref')
26+
echo "BRANCH=$PR_SOURCE_BRANCH" >> $GITHUB_ENV
27+
setup2:
28+
runs-on: ubuntu-latest
29+
steps:
30+
- name: Get PR details
31+
uses: octokit/request-action@v2.x
32+
id: get-pr-details
33+
with:
34+
route: GET /repos/{repository}/pulls/{pull_number}
35+
repository: ${{ github.repository }}
36+
pull_number: ${{ github.event.issue.number }}
37+
env:
38+
GITHUB_TOKEN: ${{ secrets.GH_TOKEN }}
39+
- name: Set environment variables
40+
run: |
41+
MERGE_STATUS=${{ fromJson(steps.get-pr-details.outputs.data).mergeable }}
42+
if $MERGE_STATUS; then echo "COMMENT=\[Fast Forward CI\] ${{ env.HEAD_REF }} cannot be merged into ${{ env.BASE_REF }} at the moment." >> $GITHUB_ENV; fi
43+
echo "MERGE_STATUS=$MERGE_STATUS" >> $GITHUB_ENV
44+
echo "BASE_REF=${{ fromJson(steps.get-pr-details.outputs.data).base.ref }}" >> $GITHUB_ENV
45+
echo "HEAD_REF=${{ fromJson(steps.get-pr-details.outputs.data).head.ref }}" >> $GITHUB_ENV
46+
setup3:
47+
runs-on: ubuntu-latest
48+
steps:
49+
- id: issues
50+
uses: octokit/request-action@v2.x
51+
with:
52+
route: GET /repos/${{ github.repository_owner }}/${{ github.repository }}/issues?state=open
53+
env:
54+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN}}
55+
- run: |
56+
echo '${{ steps.issues.outputs.data }}' > issues.json
57+
setup4:
58+
runs-on: ubuntu-latest
59+
steps:
60+
- id: get-pull-request
61+
uses: octokit/request-action@v2.x
62+
with:
63+
route: GET /repos/{owner}/{repo}/pulls/{pull_number}
64+
owner: foo
65+
repo: bar
66+
pull_number: ${{ github.event.issue.number }}
67+
68+
- run: >-
69+
echo "Pull request title is \"${{
70+
fromJson(steps.get-pull-request.outputs.data).title }}\" but expected
71+
\"Updated test pull request\"" && exit 1
72+
73+
74+

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ edges
155155
| .github/workflows/test16.yml:100:30:100:70 | steps.commit-message.outputs.value | .github/workflows/test16.yml:99:13:102:8 | Job outputs node [commit-message] | provenance | |
156156
| .github/workflows/test16.yml:123:15:128:12 | Run Step: commit-message [value] | .github/workflows/test16.yml:100:30:100:70 | steps.commit-message.outputs.value | provenance | |
157157
| .github/workflows/test16.yml:125:20:125:75 | echo "value=$(git log -1 --pretty=%s)" >> $GITHUB_OUTPUT | .github/workflows/test16.yml:123:15:128:12 | Run Step: commit-message [value] | provenance | |
158+
| .github/workflows/test17.yml:14:13:22:10 | Uses Step: get-pr | .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | provenance | |
159+
| .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | provenance | |
160+
| .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | provenance | |
161+
| .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | provenance | |
158162
| .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | provenance | |
159163
| .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | provenance | |
160164
| .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | .github/workflows/test.yml:25:18:25:48 | steps.step0.outputs.value | provenance | |
@@ -472,6 +476,14 @@ nodes
472476
| .github/workflows/test16.yml:215:19:230:24 | github.event.workflow_run.head_commit.author.name | semmle.label | github.event.workflow_run.head_commit.author.name |
473477
| .github/workflows/test16.yml:215:19:230:24 | needs.build-demo.outputs.commit-message | semmle.label | needs.build-demo.outputs.commit-message |
474478
| .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | semmle.label | needs.setup.outputs.ref |
479+
| .github/workflows/test17.yml:14:13:22:10 | Uses Step: get-pr | semmle.label | Uses Step: get-pr |
480+
| .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | semmle.label | steps.get-pr.outputs.data |
481+
| .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | semmle.label | Uses Step: get-pr-details |
482+
| .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | semmle.label | fromJson(steps.get-pr-details.outputs.data).head.ref |
483+
| .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | semmle.label | Uses Step: issues |
484+
| .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | semmle.label | steps.issues.outputs.data |
485+
| .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | semmle.label | Uses Step: get-pull-request |
486+
| .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | semmle.label | fromJson(steps.get-pull-request.outputs.data).title |
475487
| .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
476488
| .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 |
477489
| .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |
@@ -623,6 +635,10 @@ subpaths
623635
| .github/workflows/test16.yml:215:19:230:24 | needs.build-demo.outputs.commit-message | .github/workflows/test16.yml:125:20:125:75 | echo "value=$(git log -1 --pretty=%s)" >> $GITHUB_OUTPUT | .github/workflows/test16.yml:215:19:230:24 | needs.build-demo.outputs.commit-message | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test16.yml:215:19:230:24 | needs.build-demo.outputs.commit-message | ${{ needs.build-demo.outputs.commit-message }} |
624636
| .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | .github/workflows/test16.yml:26:15:33:12 | Uses Step | .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | ${{ needs.setup.outputs.ref }} |
625637
| .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | .github/workflows/test16.yml:38:15:45:12 | Uses Step | .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | ${{ needs.setup.outputs.ref }} |
638+
| .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | .github/workflows/test17.yml:14:13:22:10 | Uses Step: get-pr | .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | ${{ steps.get-pr.outputs.data }} |
639+
| .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | ${{ fromJson(steps.get-pr-details.outputs.data).head.ref }} |
640+
| .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | ${{ steps.issues.outputs.data }} |
641+
| .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | ${{ fromJson(steps.get-pull-request.outputs.data).title }} |
626642
| .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | .github/workflows/test.yml:20:20:20:62 | github.event['pull_request']['body'] | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | ${{needs.job1.outputs['job_output']}} |
627643
| .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | ${{ steps.artifact.outputs.pr_number }} |
628644
| .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 }} |

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ edges
155155
| .github/workflows/test16.yml:100:30:100:70 | steps.commit-message.outputs.value | .github/workflows/test16.yml:99:13:102:8 | Job outputs node [commit-message] | provenance | |
156156
| .github/workflows/test16.yml:123:15:128:12 | Run Step: commit-message [value] | .github/workflows/test16.yml:100:30:100:70 | steps.commit-message.outputs.value | provenance | |
157157
| .github/workflows/test16.yml:125:20:125:75 | echo "value=$(git log -1 --pretty=%s)" >> $GITHUB_OUTPUT | .github/workflows/test16.yml:123:15:128:12 | Run Step: commit-message [value] | provenance | |
158+
| .github/workflows/test17.yml:14:13:22:10 | Uses Step: get-pr | .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | provenance | |
159+
| .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | provenance | |
160+
| .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | provenance | |
161+
| .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | provenance | |
158162
| .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | provenance | |
159163
| .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | provenance | |
160164
| .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | .github/workflows/test.yml:25:18:25:48 | steps.step0.outputs.value | provenance | |
@@ -472,6 +476,14 @@ nodes
472476
| .github/workflows/test16.yml:215:19:230:24 | github.event.workflow_run.head_commit.author.name | semmle.label | github.event.workflow_run.head_commit.author.name |
473477
| .github/workflows/test16.yml:215:19:230:24 | needs.build-demo.outputs.commit-message | semmle.label | needs.build-demo.outputs.commit-message |
474478
| .github/workflows/test16.yml:215:19:230:24 | needs.setup.outputs.ref | semmle.label | needs.setup.outputs.ref |
479+
| .github/workflows/test17.yml:14:13:22:10 | Uses Step: get-pr | semmle.label | Uses Step: get-pr |
480+
| .github/workflows/test17.yml:25:41:25:72 | steps.get-pr.outputs.data | semmle.label | steps.get-pr.outputs.data |
481+
| .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | semmle.label | Uses Step: get-pr-details |
482+
| .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | semmle.label | fromJson(steps.get-pr-details.outputs.data).head.ref |
483+
| .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | semmle.label | Uses Step: issues |
484+
| .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | semmle.label | steps.issues.outputs.data |
485+
| .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | semmle.label | Uses Step: get-pull-request |
486+
| .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | semmle.label | fromJson(steps.get-pull-request.outputs.data).title |
475487
| .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
476488
| .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 |
477489
| .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |

0 commit comments

Comments
 (0)