Skip to content

Commit e03ba55

Browse files
author
Alvaro Muñoz
committed
Account for checkout path on Untrusted Checkout Critical
1 parent 7cba2e0 commit e03ba55

5 files changed

Lines changed: 347 additions & 7 deletions

File tree

‎ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,12 @@ class ArtifactPoisoningSink extends DataFlow::Node {
276276
)
277277
or
278278
poisonable.(UsesStep) = this.asExpr() and
279-
download.getPath() = "GITHUB_WORKSPACE/"
279+
(
280+
not poisonable instanceof LocalActionUsesStep and
281+
download.getPath() = "GITHUB_WORKSPACE/"
282+
or
283+
isSubpath(poisonable.(LocalActionUsesStep).getPath(), download.getPath())
284+
)
280285
)
281286
}
282287

‎ql/lib/codeql/actions/security/PoisonableSteps.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,6 @@ class LocalScriptExecutionRunStep extends PoisonableStep, Run {
4949

5050
class LocalActionUsesStep extends PoisonableStep, UsesStep {
5151
LocalActionUsesStep() { this.getCallee().matches("./%") }
52+
53+
string getPath() { result = normalizePath(this.getCallee()) }
5254
}

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

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,33 @@ import codeql.actions.security.ControlChecks
2020

2121
query predicate edges(Step a, Step b) { a.getNextStep() = b }
2222

23-
from PRHeadCheckoutStep checkout, PoisonableStep step, Event event
23+
from PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event
2424
where
2525
// the checkout is followed by a known poisonable step
26-
checkout.getAFollowingStep() = step and
26+
checkout.getAFollowingStep() = poisonable and
27+
(
28+
poisonable instanceof Run and
29+
(
30+
// Check if the poisonable step is a local script execution step
31+
// and the path of the command or script matches the path of the downloaded artifact
32+
isSubpath(poisonable.(LocalScriptExecutionRunStep).getPath(), checkout.getPath())
33+
or
34+
// Checking the path for non local script execution steps is very difficult
35+
not poisonable instanceof LocalScriptExecutionRunStep
36+
// Its not easy to extract the path from a non-local script execution step so skipping this check for now
37+
// and isSubpath(poisonable.(Run).getWorkingDirectory(), checkout.getPath())
38+
)
39+
or
40+
poisonable instanceof UsesStep and
41+
(
42+
not poisonable instanceof LocalActionUsesStep and
43+
checkout.getPath() = "GITHUB_WORKSPACE/"
44+
or
45+
isSubpath(poisonable.(LocalActionUsesStep).getPath(), checkout.getPath())
46+
)
47+
) and
2748
// the checkout occurs in a privileged context
28-
inPrivilegedContext(step, event) and
29-
not exists(ControlCheck check | check.protects(step, event, "untrusted-checkout"))
30-
select step, checkout, step, "Execution of untrusted code on a privileged workflow. $@", event,
31-
event.getLocation().getFile().toString()
49+
inPrivilegedContext(poisonable, event) and
50+
not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout"))
51+
select poisonable, checkout, poisonable, "Execution of untrusted code on a privileged workflow. $@",
52+
event, event.getLocation().getFile().toString()

‎ql/test/query-tests/Security/CWE-829/.github/workflows/test16.yml‎

Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
name: Post-Build
2+
run-name: Post-Build on ${{ github.event.workflow_run.head_branch }}
3+
on:
4+
workflow_run:
5+
types: [ 'completed' ]
6+
workflows:
7+
- Build
8+
concurrency:
9+
# Cancel concurrent jobs on pull_request but not push, by including the run_id in the concurrency group for the latter.
10+
group: post-build-${{ github.event.workflow_run.event == 'push' && github.run_id || 'pr' }}-${{ github.event.workflow_run.head_branch }}
11+
cancel-in-progress: true
12+
13+
env:
14+
COMPOSER_ROOT_VERSION: "dev-trunk"
15+
SUMMARY: Post-Build run [#${{ github.run_id }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for Build run [#${{ github.event.workflow_run.id }}](${{ github.event.workflow_run.html_url }})
16+
17+
permissions:
18+
actions: read
19+
contents: read
20+
pull-requests: read
21+
22+
# Note the job logic here is a bit unusual. That's because this workflow is triggered by `workflow_run`, and so is not shown on the PR by default.
23+
# Instead we have to manually report back, including where we could normally just skip or let a failure be handled.
24+
# - If the "Build" job failed, we need to set our status as failed too (build_failed).
25+
# - If the find_artifact job fails for some reason, we need a step to explicitly report that back.
26+
# - If no plugins are found, we need to explicitly report back a "skipped" status.
27+
# - And the upgrade_test job both explicitly sets "in progress" at its start and updates at its end.
28+
#
29+
# If you're wanting to add a new check, you'd want to do the following:
30+
# - Add a step in the `setup` workflow to create your check, and a corresponding output for later steps to have the ID.
31+
# - Add a step in the `build_failed` workflow to set your run to cancelled.
32+
# - Add a job to run whatever tests you need to run, with steps similar to the `upgrade_test` workflow's "Get token", "Notify check in progress", and "Notify final status".
33+
# - Add a step in the `no_plugins` workflow to set your run to skipped if your job only runs when there are plugins built.
34+
35+
jobs:
36+
setup:
37+
name: Setup
38+
runs-on: ubuntu-latest
39+
timeout-minutes: 2 # 2022-12-20: Seems like it should be fast.
40+
outputs:
41+
upgrade_check: ${{ steps.upgrade_check.outputs.id }}
42+
steps:
43+
- name: Log info
44+
run: |
45+
echo "$SUMMARY" >> $GITHUB_STEP_SUMMARY
46+
47+
- uses: actions/checkout@v4
48+
49+
- name: Get token
50+
id: get_token
51+
uses: ./.github/actions/gh-app-token
52+
with:
53+
app_id: ${{ secrets.JP_LAUNCH_CONTROL_ID }}
54+
private_key: ${{ secrets.JP_LAUNCH_CONTROL_KEY }}
55+
56+
- name: 'Create "Test plugin upgrades" check'
57+
id: upgrade_check
58+
uses: ./.github/actions/check-run
59+
with:
60+
name: Test plugin upgrades
61+
sha: ${{ github.event.workflow_run.head_sha }}
62+
status: queued
63+
title: Test queued...
64+
summary: |
65+
${{ env.SUMMARY }}
66+
token: ${{ steps.get_token.outputs.token }}
67+
68+
build_failed:
69+
name: Handle build failure
70+
runs-on: ubuntu-latest
71+
needs: setup
72+
if: github.event.workflow_run.conclusion != 'success'
73+
timeout-minutes: 2 # 2022-08-26: Seems like it should be fast.
74+
steps:
75+
- uses: actions/checkout@v4
76+
77+
- name: Get token
78+
id: get_token
79+
uses: ./.github/actions/gh-app-token
80+
with:
81+
app_id: ${{ secrets.JP_LAUNCH_CONTROL_ID }}
82+
private_key: ${{ secrets.JP_LAUNCH_CONTROL_KEY }}
83+
84+
- name: 'Mark "Test plugin upgrades" cancelled'
85+
uses: ./.github/actions/check-run
86+
with:
87+
id: ${{ needs.setup.outputs.upgrade_check }}
88+
conclusion: cancelled
89+
title: Build failed
90+
summary: |
91+
${{ env.SUMMARY }}
92+
93+
Post-build run aborted because the build did not succeed.
94+
token: ${{ steps.get_token.outputs.token }}
95+
96+
find_artifact:
97+
name: Find artifact
98+
runs-on: ubuntu-latest
99+
needs: setup
100+
if: github.event.workflow_run.conclusion == 'success'
101+
timeout-minutes: 2 # 2022-08-26: Seems like it should be fast.
102+
outputs:
103+
zip_url: ${{ steps.run.outputs.zip_url }}
104+
any_plugins: ${{ steps.run.outputs.any_plugins }}
105+
steps:
106+
- uses: actions/checkout@v4
107+
108+
- name: Find artifact
109+
id: run
110+
env:
111+
TOKEN: ${{ github.token }}
112+
URL: ${{ github.event.workflow_run.artifacts_url }}
113+
run: |
114+
for (( i=1; i<=5; i++ )); do
115+
[[ $i -gt 1 ]] && sleep 10
116+
echo "::group::Fetch list of artifacts (attempt $i/5)"
117+
JSON="$(curl -v -L --get \
118+
--header "Authorization: token $TOKEN" \
119+
--url "$URL"
120+
)"
121+
echo "$JSON"
122+
echo "::endgroup::"
123+
ZIPURL="$(jq -r '.artifacts | map( select( .name == "jetpack-build" ) ) | sort_by( .created_at ) | last | .archive_download_url // empty' <<<"$JSON")"
124+
PLUGINS="$(jq -r '.artifacts[] | select( .name == "plugins.tsv" )' <<<"$JSON")"
125+
if [[ -n "$ZIPURL" ]]; then
126+
break
127+
fi
128+
done
129+
[[ -z "$ZIPURL" ]] && { echo "::error::Failed to find artifact."; exit 1; }
130+
echo "Zip URL: $ZIPURL"
131+
echo "zip_url=${ZIPURL}" >> "$GITHUB_OUTPUT"
132+
if [[ -z "$PLUGINS" ]]; then
133+
echo "Any plugins? No"
134+
echo "any_plugins=false" >> "$GITHUB_OUTPUT"
135+
else
136+
echo "Any plugins? Yes"
137+
echo "any_plugins=true" >> "$GITHUB_OUTPUT"
138+
fi
139+
140+
- name: Get token
141+
id: get_token
142+
if: ${{ ! success() }}
143+
uses: ./.github/actions/gh-app-token
144+
with:
145+
app_id: ${{ secrets.JP_LAUNCH_CONTROL_ID }}
146+
private_key: ${{ secrets.JP_LAUNCH_CONTROL_KEY }}
147+
- name: 'Mark "Test plugin upgrades" failed'
148+
if: ${{ ! success() }}
149+
uses: ./.github/actions/check-run
150+
with:
151+
id: ${{ needs.setup.outputs.upgrade_check }}
152+
conclusion: failure
153+
title: Failed to find build artifact
154+
summary: |
155+
${{ env.SUMMARY }}
156+
157+
Post-build run aborted because the "Find artifact" step failed.
158+
token: ${{ steps.get_token.outputs.token }}
159+
160+
no_plugins:
161+
name: Handle no-plugins
162+
runs-on: ubuntu-latest
163+
needs: [ setup, find_artifact ]
164+
if: needs.find_artifact.outputs.any_plugins == 'false'
165+
timeout-minutes: 2 # 2022-08-26: Seems like it should be fast.
166+
steps:
167+
- uses: actions/checkout@v4
168+
169+
- name: Get token
170+
id: get_token
171+
uses: ./.github/actions/gh-app-token
172+
with:
173+
app_id: ${{ secrets.JP_LAUNCH_CONTROL_ID }}
174+
private_key: ${{ secrets.JP_LAUNCH_CONTROL_KEY }}
175+
176+
- name: 'Mark "Test plugin upgrades" skipped'
177+
uses: ./.github/actions/check-run
178+
with:
179+
id: ${{ needs.setup.outputs.upgrade_check }}
180+
conclusion: skipped
181+
title: No plugins were built
182+
summary: |
183+
${{ env.SUMMARY }}
184+
185+
Post-build run skipped because no plugins were built.
186+
token: ${{ steps.get_token.outputs.token }}
187+
188+
upgrade_test:
189+
name: Test plugin upgrades
190+
runs-on: ubuntu-latest
191+
needs: [ setup, find_artifact ]
192+
if: needs.find_artifact.outputs.any_plugins == 'true'
193+
timeout-minutes: 15 # 2022-08-26: Successful runs seem to take about 6 minutes, but give some extra time for the downloads.
194+
services:
195+
db:
196+
image: mariadb:lts
197+
env:
198+
MARIADB_ROOT_PASSWORD: wordpress
199+
ports:
200+
- 3306:3306
201+
options: --health-cmd="healthcheck.sh --su-mysql --connect --innodb_initialized" --health-interval=10s --health-timeout=5s --health-retries=5
202+
container:
203+
image: ghcr.io/automattic/jetpack-wordpress-dev:latest
204+
env:
205+
WP_DOMAIN: localhost
206+
WP_ADMIN_USER: wordpress
207+
WP_ADMIN_EMAIL: wordpress@example.com
208+
WP_ADMIN_PASSWORD: wordpress
209+
WP_TITLE: Hello World
210+
MYSQL_HOST: db:3306
211+
MYSQL_DATABASE: wordpress
212+
MYSQL_USER: root
213+
MYSQL_PASSWORD: wordpress
214+
HOST_PORT: 80
215+
ports:
216+
- 80:80
217+
steps:
218+
- uses: actions/checkout@v4
219+
with:
220+
path: trunk
221+
- uses: actions/checkout@v4
222+
with:
223+
ref: ${{ github.event.workflow_run.head_commit.id }}
224+
path: commit
225+
226+
- name: Get token
227+
id: get_token
228+
uses: ./trunk/.github/actions/gh-app-token
229+
env:
230+
# Work around a weird node 16/openssl 3 issue in the docker env
231+
OPENSSL_CONF: '/dev/null'
232+
with:
233+
app_id: ${{ secrets.JP_LAUNCH_CONTROL_ID }}
234+
private_key: ${{ secrets.JP_LAUNCH_CONTROL_KEY }}
235+
236+
- name: Notify check in progress
237+
uses: ./trunk/.github/actions/check-run
238+
with:
239+
id: ${{ needs.setup.outputs.upgrade_check }}
240+
status: in_progress
241+
title: Test started...
242+
summary: |
243+
${{ env.SUMMARY }}
244+
245+
See run [#${{ github.run_id }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.
246+
token: ${{ steps.get_token.outputs.token }}
247+
248+
- name: Download build artifact
249+
env:
250+
TOKEN: ${{ github.token }}
251+
ZIPURL: ${{ needs.find_artifact.outputs.zip_url }}
252+
shell: bash
253+
run: |
254+
for (( i=1; i<=2; i++ )); do
255+
[[ $i -gt 1 ]] && sleep 10
256+
echo "::group::Downloading artifact (attempt $i/2)"
257+
curl -v -L --get \
258+
--header "Authorization: token $TOKEN" \
259+
--url "$ZIPURL" \
260+
--output "artifact.zip"
261+
echo "::endgroup::"
262+
if [[ -e "artifact.zip" ]] && zipinfo artifact.zip &>/dev/null; then
263+
break
264+
fi
265+
done
266+
[[ ! -e "artifact.zip" ]] && { echo "::error::Failed to download artifact."; exit 1; }
267+
unzip artifact.zip
268+
tar --xz -xvvf build.tar.xz build
269+
270+
- name: Setup WordPress
271+
run: trunk/.github/files/test-plugin-update/setup.sh
272+
273+
- name: Prepare plugin zips
274+
id: zips
275+
run: trunk/.github/files/test-plugin-update/prepare-zips.sh
276+
277+
- name: Test upgrades
278+
id: tests
279+
run: trunk/.github/files/test-plugin-update/test.sh
280+
281+
- name: Notify final status
282+
if: always()
283+
uses: ./trunk/.github/actions/check-run
284+
with:
285+
id: ${{ needs.setup.outputs.upgrade_check }}
286+
conclusion: ${{ job.status }}
287+
title: ${{ job.status == 'success' && 'Tests passed' || job.status == 'cancelled' && 'Cancelled' || 'Tests failed' }}
288+
summary: |
289+
${{ env.SUMMARY }}
290+
291+
${{ steps.zips.outputs.info }}${{ steps.tests.outputs.info }}
292+
293+
See run [#${{ github.run_id }}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.
294+
token: ${{ steps.get_token.outputs.token }}

‎ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,24 @@ edges
244244
| .github/workflows/test15.yml:236:7:242:4 | Run Step | .github/workflows/test15.yml:242:7:250:4 | Uses Step |
245245
| .github/workflows/test15.yml:242:7:250:4 | Uses Step | .github/workflows/test15.yml:250:7:270:4 | Uses Step |
246246
| .github/workflows/test15.yml:250:7:270:4 | Uses Step | .github/workflows/test15.yml:270:7:271:45 | Run Step |
247+
| .github/workflows/test16.yml:43:9:47:6 | Run Step | .github/workflows/test16.yml:47:9:49:6 | Uses Step |
248+
| .github/workflows/test16.yml:47:9:49:6 | Uses Step | .github/workflows/test16.yml:49:9:56:6 | Uses Step: get_token |
249+
| .github/workflows/test16.yml:49:9:56:6 | Uses Step: get_token | .github/workflows/test16.yml:56:9:68:2 | Uses Step: upgrade_check |
250+
| .github/workflows/test16.yml:75:9:77:6 | Uses Step | .github/workflows/test16.yml:77:9:84:6 | Uses Step: get_token |
251+
| .github/workflows/test16.yml:77:9:84:6 | Uses Step: get_token | .github/workflows/test16.yml:84:9:96:2 | Uses Step |
252+
| .github/workflows/test16.yml:106:9:108:6 | Uses Step | .github/workflows/test16.yml:108:9:140:6 | Run Step: run |
253+
| .github/workflows/test16.yml:108:9:140:6 | Run Step: run | .github/workflows/test16.yml:140:9:147:6 | Uses Step: get_token |
254+
| .github/workflows/test16.yml:140:9:147:6 | Uses Step: get_token | .github/workflows/test16.yml:147:9:160:2 | Uses Step |
255+
| .github/workflows/test16.yml:167:9:169:6 | Uses Step | .github/workflows/test16.yml:169:9:176:6 | Uses Step: get_token |
256+
| .github/workflows/test16.yml:169:9:176:6 | Uses Step: get_token | .github/workflows/test16.yml:176:9:188:2 | Uses Step |
257+
| .github/workflows/test16.yml:218:9:221:6 | Uses Step | .github/workflows/test16.yml:221:9:226:6 | Uses Step |
258+
| .github/workflows/test16.yml:221:9:226:6 | Uses Step | .github/workflows/test16.yml:226:9:236:6 | Uses Step: get_token |
259+
| .github/workflows/test16.yml:226:9:236:6 | Uses Step: get_token | .github/workflows/test16.yml:236:9:248:6 | Uses Step |
260+
| .github/workflows/test16.yml:236:9:248:6 | Uses Step | .github/workflows/test16.yml:248:9:270:6 | Run Step |
261+
| .github/workflows/test16.yml:248:9:270:6 | Run Step | .github/workflows/test16.yml:270:9:273:6 | Run Step |
262+
| .github/workflows/test16.yml:270:9:273:6 | Run Step | .github/workflows/test16.yml:273:9:277:6 | Run Step: zips |
263+
| .github/workflows/test16.yml:273:9:277:6 | Run Step: zips | .github/workflows/test16.yml:277:9:281:6 | Run Step: tests |
264+
| .github/workflows/test16.yml:277:9:281:6 | Run Step: tests | .github/workflows/test16.yml:281:9:294:54 | Uses Step |
247265
| .github/workflows/test.yml:13:9:14:6 | Uses Step | .github/workflows/test.yml:14:9:25:6 | Run Step |
248266
| .github/workflows/test.yml:14:9:25:6 | Run Step | .github/workflows/test.yml:25:9:33:6 | Run Step |
249267
| .github/workflows/test.yml:25:9:33:6 | Run Step | .github/workflows/test.yml:33:9:37:34 | Run Step |

0 commit comments

Comments
 (0)