Skip to content

Commit 9d5b026

Browse files
author
Alvaro Muñoz
committed
2 parents 06747cd + b6a097c commit 9d5b026

File tree

5 files changed

+136
-5
lines changed

5 files changed

+136
-5
lines changed

‎ql/src/Security/CWE-829/UntrustedCheckout.ql

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* @name Checkout of untrusted code in trusted context
3-
* @description Workflows triggered on `pull_request_target` have read/write access to the base repository and access to secrets.
3+
* @description Priveleged workflows have read/write access to the base repository and access to secrets.
44
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
55
* that is able to push to the base repository and to access secrets.
66
* @kind problem
@@ -121,12 +121,26 @@ class GitCheckout extends PRHeadCheckoutStep instanceof Run {
121121
}
122122
}
123123

124+
predicate isSingleTriggerWorkflow(Workflow w, string trigger) {
125+
w.getATriggerEvent() = trigger and
126+
count(string t | w.getATriggerEvent() = t | t) = 1
127+
}
128+
124129
from Workflow w, PRHeadCheckoutStep checkout
125130
where
126-
w.hasTriggerEvent([
127-
"pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review",
128-
"workflow_run", "check_run", "check_suite", "workflow_call"
129-
]) and
131+
(
132+
// The Workflow is triggered by an event other than `pull_request`
133+
not isSingleTriggerWorkflow(w, "pull_request")
134+
or
135+
// The Workflow is only triggered by `workflow_call` and there is
136+
// a caller workflow triggered by an event other than `pull_request`
137+
isSingleTriggerWorkflow(w, "workflow_call") and
138+
exists(ExternalJob call, Workflow caller |
139+
call.getCallee() = w.getLocation().getFile().getRelativePath() and
140+
caller = call.getWorkflow() and
141+
not isSingleTriggerWorkflow(caller, "pull_request")
142+
)
143+
) and
130144
w.getAJob().(LocalJob).getAStep() = checkout and
131145
not exists(ControlCheck check |
132146
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
name: changelog
2+
3+
on:
4+
workflow_call:
5+
inputs:
6+
create:
7+
description: Add a log to the changelog
8+
type: boolean
9+
required: false
10+
default: false
11+
update:
12+
description: Update the existing changelog
13+
type: boolean
14+
required: false
15+
default: false
16+
17+
jobs:
18+
changelog:
19+
runs-on: ubuntu-latest
20+
env:
21+
file: CHANGELOG.md
22+
steps:
23+
- uses: actions/checkout@v3
24+
with:
25+
fetch-depth: 0
26+
- name: Check ${{ env.file }}
27+
run: |
28+
if [[ $(git diff --name-only origin/master HEAD -- ${{ env.file }} | grep '^${{ env.file }}$' -c) -eq 0 ]]; then
29+
echo "Expected '${{ env.file }}' to be modified"
30+
exit 1
31+
fi
32+
update:
33+
runs-on: ubuntu-latest
34+
needs: changelog
35+
if: (inputs.create && failure()) || (inputs.update && success())
36+
continue-on-error: true
37+
env:
38+
file: CHANGELOG.md
39+
next_version: next
40+
link: '[#${{ github.event.number }}](https://github.com/fabricjs/fabric.js/pull/${{ github.event.number }})'
41+
steps:
42+
- uses: actions/checkout@v3
43+
with:
44+
ref: ${{ github.event.pull_request.head.ref }}
45+
- name: Update ${{ env.file }} from PR title
46+
id: update
47+
uses: actions/github-script@v6
48+
env:
49+
log: '- ${{ github.event.pull_request.title }} ${{ env.link }}\n'
50+
prev_log: '- ${{ github.event.changes.title.from }} ${{ env.link }}\n'
51+
with:
52+
result-encoding: string
53+
script: |
54+
const fs = require('fs');
55+
const file = './${{ env.file }}';
56+
let content = fs.readFileSync(file).toString();
57+
const title = '[${{ env.next_version }}]';
58+
const log = '${{ env.log }}';
59+
let exists = ${{ needs.changelog.result == 'success' }};
60+
61+
if (!content.includes(title)) {
62+
const insertAt = content.indexOf('\n') + 1;
63+
content =
64+
content.slice(0, insertAt) +
65+
`\n## ${title}\n\n\n` +
66+
content.slice(insertAt);
67+
}
68+
69+
const insertAt = content.indexOf('\n', content.indexOf(title) + title.length + 1) + 1;
70+
if (exists && ${{ github.event.action == 'edited' }}) {
71+
const prevLog = '${{ env.prev_log }}';
72+
const index = content.indexOf(prevLog, insertAt);
73+
if (index > -1) {
74+
content = content.slice(0, index) + content.slice(index + prevLog.length);
75+
exists = false;
76+
}
77+
}
78+
79+
if (!exists) {
80+
content = content.slice(0, insertAt) + log + content.slice(insertAt);
81+
fs.writeFileSync(file, content);
82+
return true;
83+
}
84+
85+
return false;
86+
- name: Setup node
87+
if: fromJson(steps.update.outputs.result)
88+
uses: actions/setup-node@v3
89+
with:
90+
node-version: 18.x
91+
- name: Commit & Push
92+
if: fromJson(steps.update.outputs.result)
93+
run: |
94+
npm ci
95+
npx prettier --write ${{ env.file }}
96+
git config user.name github-actions[bot]
97+
git config user.email github-actions[bot]@users.noreply.github.com
98+
git add ${{ env.file }}
99+
git commit -m "update ${{ env.file }}"
100+
git push

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
name: '📋'
2+
3+
on:
4+
pull_request_target:
5+
branches: [master]
6+
7+
jobs:
8+
changelog:
9+
uses: ./.github/workflows/changelog_from_prt.yml

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ edges
44
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] |
55
| .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files |
66
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
7+
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
78
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
89
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced |
910
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] |
@@ -66,6 +67,8 @@ nodes
6667
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files |
6768
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
6869
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
70+
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
71+
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log |
6972
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body |
7073
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body |
7174
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body |
@@ -205,6 +208,7 @@ subpaths
205208
| .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} |
206209
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | ${{ steps.changed-files.outputs.all_changed_files }} |
207210
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
211+
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
208212
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} |
209213
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} |
210214
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ edges
44
| .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] |
55
| .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files |
66
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
7+
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
78
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
89
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced |
910
| .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] |
@@ -66,6 +67,8 @@ nodes
6667
| .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files |
6768
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
6869
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
70+
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
71+
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log |
6972
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body |
7073
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body |
7174
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body |
@@ -204,6 +207,7 @@ subpaths
204207
#select
205208
| .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} |
206209
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
210+
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
207211
| .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} |
208212
| .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} |
209213
| .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} |

0 commit comments

Comments
 (0)