Skip to content

Commit 35df951

Browse files
author
Alvaro Muñoz
committed
Support more untrusted checkout cases
1 parent 9ca1ac5 commit 35df951

File tree

7 files changed

+293
-7
lines changed

7 files changed

+293
-7
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
import actions
17+
import codeql.actions.DataFlow
1718

1819
/** An If node that contains an actor, user or label check */
1920
class ControlCheck extends If {
@@ -23,6 +24,7 @@ class ControlCheck extends If {
2324
.regexpFind([
2425
"\\bgithub\\.actor\\b", // actor
2526
"\\bgithub\\.triggering_actor\\b", // actor
27+
"\\bgithub\\.event\\.comment\\.user\\.login\\b", //user
2628
"\\bgithub\\.event\\.pull_request\\.user\\.login\\b", //user
2729
"\\bgithub\\.event\\.pull_request\\.labels\\b", // label
2830
"\\bgithub\\.event\\.label\\.name\\b" // label
@@ -47,22 +49,18 @@ predicate containsHeadRef(string s) {
4749
"\\bgithub\\.event\\.workflow_run\\.head_branch\\b", // The branch of the head commit.
4850
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.id\\b", // The SHA of the head commit.
4951
"\\bgithub\\.event\\.workflow_run\\.head_sha\\b", // The SHA of the head commit.
50-
"\\benv\\.GITHUB_HEAD_REF\\b",
51-
52-
"\\bgithub\\.event\\.check_suite\\.after\\b",
52+
"\\benv\\.GITHUB_HEAD_REF\\b", "\\bgithub\\.event\\.check_suite\\.after\\b",
5353
"\\bgithub\\.event\\.check_suite\\.head_sha\\b",
5454
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
5555
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
5656
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
5757
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
58-
5958
"\\bgithub\\.event\\.check_run\\.check_suite\\.after\\b",
6059
"\\bgithub\\.event\\.check_run\\.check_suite\\.head_sha\\b",
6160
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
6261
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
6362
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
6463
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
65-
6664
"\\bgithub\\.event\\.check_run\\.head_sha\\b",
6765
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
6866
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
@@ -79,7 +77,14 @@ abstract class PRHeadCheckoutStep extends Step { }
7977
class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep {
8078
ActionsCheckout() {
8179
this.getCallee() = "actions/checkout" and
82-
containsHeadRef(this.getArgumentExpr("ref").getExpression())
80+
(
81+
containsHeadRef(this.getArgumentExpr("ref").getExpression())
82+
or
83+
exists(UsesStep head |
84+
head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and
85+
DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref"))
86+
)
87+
)
8388
}
8489
}
8590

@@ -103,7 +108,10 @@ class GitCheckout extends PRHeadCheckoutStep instanceof Run {
103108

104109
from Workflow w, PRHeadCheckoutStep checkout
105110
where
106-
w.hasTriggerEvent(["pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review", "workflow_run", "check_run", "check_suite", "workflow_call"]) and
111+
w.hasTriggerEvent([
112+
"pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review",
113+
"workflow_run", "check_run", "check_suite", "workflow_call"
114+
]) and
107115
w.getAJob().(LocalJob).getAStep() = checkout and
108116
not exists(ControlCheck check |
109117
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
name: PR head from 3rd party action
2+
3+
on:
4+
workflow_call:
5+
workflow_dispatch:
6+
7+
jobs:
8+
9+
test1:
10+
runs-on: ubuntu-20.04
11+
steps:
12+
- name: (PR comment) Get PR branch
13+
if: ${{ github.event_name == 'issue_comment' }}
14+
uses: xt0rted/pull-request-comment-branch@v2
15+
id: comment-branch
16+
17+
- name: (PR comment) Checkout PR branch
18+
if: ${{ github.event_name == 'issue_comment' }}
19+
uses: actions/checkout@v3
20+
with:
21+
ref: ${{ steps.comment-branch.outputs.head_sha }}
22+
23+
test2:
24+
runs-on: ubuntu-20.04
25+
steps:
26+
- name: (PR comment) Get PR branch
27+
if: ${{ github.event_name == 'issue_comment' }}
28+
uses: xt0rted/pull-request-comment-branch@v2
29+
id: comment-branch
30+
31+
- name: (PR comment) Checkout PR branch
32+
if: ${{ github.event_name == 'issue_comment' }}
33+
uses: actions/checkout@v3
34+
with:
35+
ref: ${{ steps.comment-branch.outputs.head_ref }}
36+
37+
test3:
38+
runs-on: ubuntu-20.04
39+
steps:
40+
- name: resolve pr refs
41+
id: refs
42+
uses: eficode/resolve-pr-refs@main
43+
with:
44+
token: ${{ secrets.GITHUB_TOKEN }}
45+
46+
- uses: actions/checkout@v4
47+
with:
48+
ref: ${{ steps.refs.outputs.head_ref }}
49+
fetch-depth: 0
50+
- uses: actions/checkout@v4
51+
with:
52+
ref: ${{ steps.refs.outputs.head_sha }}
53+
fetch-depth: 0

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
name: Direct access
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
jobs:
8+
test1:
9+
runs-on: ubuntu-latest
10+
if: github.event_name == 'issue_comment' && github.event.issue.pull_request
11+
steps:
12+
- name: Unsafe Code Checkout
13+
uses: actions/checkout@v4
14+
with:
15+
ref: ${{ github.event.pull_request.head.ref || github.head_ref }} # Checkout the branch that made the PR or the comment's PR branch
16+
test2:
17+
runs-on: ubuntu-latest
18+
if: github.event.issue.pull_request && github.event.comment.body == '/trigger release'
19+
steps:
20+
- uses: actions/checkout@v4
21+
with:
22+
ref: refs/pull/${{ github.event.issue.number }}/merge
23+
24+
test3:
25+
runs-on: ubuntu-latest
26+
if: github.event.issue.pull_request && github.event.comment.body == '/trigger release'
27+
steps:
28+
- uses: actions/checkout@v4
29+
with:
30+
ref: ${{ format('refs/pull/{0}/merge', github.event.issue.number) }}
31+
32+
test4:
33+
runs-on: ubuntu-latest
34+
steps:
35+
- name: Checkout Branch
36+
uses: actions/checkout@v4
37+
with:
38+
ref: ${{ (github.event_name == 'pull_request_review_comment') && format('refs/pull/{0}/merge', github.event.pull_request.number) || '' }}
39+
40+
test5:
41+
runs-on: ubuntu-latest
42+
steps:
43+
- name: Checkout Branch
44+
uses: actions/checkout@v4
45+
with:
46+
ref: ${{ github.event_name == 'issue_comment' && format('refs/pull/{0}/merge', github.event.issue.number) || '' }}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
name: Heuristic based
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
jobs:
8+
test1:
9+
runs-on: ubuntu-latest
10+
steps:
11+
- name: Get Info from comment
12+
uses: actions/github-script@v7
13+
id: get-pr-info
14+
with:
15+
script: |
16+
const request = {
17+
owner: context.repo.owner,
18+
repo: context.repo.repo,
19+
pull_number: ${{ github.event.issue.number }},
20+
}
21+
core.info(`Getting PR #${request.pull_number} from ${request.owner}/${request.repo}`)
22+
const pr = await github.rest.pulls.get(request);
23+
return pr.data;
24+
- name: Debug
25+
id: get-sha
26+
run: |
27+
echo "sha=${{ fromJSON(steps.get-pr-info.outputs.result).head.sha }}" >> $GITHUB_OUTPUT
28+
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} : ${{steps.get-sha.outputs.sha}} )"
29+
uses: actions/checkout@v4
30+
with:
31+
ref: ${{ steps.get-sha.outputs.sha }}
32+
33+
test2:
34+
runs-on: ubuntu-latest
35+
36+
steps:
37+
- name: Detect branch for PR
38+
id: vars
39+
run: |
40+
PR=$( echo "${{ github.event.comment.issue_url }}" | grep -oE 'issues/([0-9]+)$' | cut -d'/' -f 2 )
41+
PR_INFO=$( curl \
42+
--request GET \
43+
--header 'authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' \
44+
--header 'content-type: application/json' \
45+
--url https://api.github.com/repos/$GITHUB_REPOSITORY/pulls/$PR )
46+
REF=$(echo "${PR_INFO}" | jq -r .head.ref)
47+
echo "branch=$REF" >> $GITHUB_OUTPUT
48+
- uses: actions/checkout@v4
49+
with:
50+
ref: ${{ steps.vars.outputs.branch }}

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
name: Octokit (heuristics)
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
jobs:
8+
test1:
9+
if: github.event.comment.body == '@metabase-bot run visual tests'
10+
runs-on: ubuntu-22.04
11+
steps:
12+
- name: Fetch issue
13+
uses: octokit/[email protected]
14+
id: fetch_issue
15+
with:
16+
route: GET ${{ github.event.issue.url }}
17+
env:
18+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
19+
- name: Fetch PR
20+
uses: octokit/[email protected]
21+
id: fetch_pr
22+
with:
23+
route: GET ${{ fromJson(steps.fetch_issue.outputs.data).pull_request.url }}
24+
env:
25+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
26+
- uses: actions/checkout@v4
27+
with:
28+
ref: ${{ fromJson(steps.fetch_pr.outputs.data).head.ref }}
29+
token: ${{ secrets.GITHUB_TOKEN }}
30+
- uses: actions/checkout@v4
31+
with:
32+
ref: ${{ fromJson(steps.fetch_pr.outputs.data).head.sha }}
33+
token: ${{ secrets.GITHUB_TOKEN }}
34+
35+
test2:
36+
runs-on: ubuntu-latest
37+
steps:
38+
- name: Get Info from comment
39+
uses: actions/github-script@v7
40+
id: get-pr-info
41+
with:
42+
script: |
43+
const request = {
44+
owner: context.repo.owner,
45+
repo: context.repo.repo,
46+
pull_number: ${{ github.event.issue.number }},
47+
}
48+
core.info(`Getting PR #${request.pull_number} from ${request.owner}/${request.repo}`)
49+
const pr = await github.rest.pulls.get(request);
50+
return pr.data;
51+
52+
- name: Debug
53+
id: get-sha
54+
run: |
55+
echo "sha=${{ fromJSON(steps.get-pr-info.outputs.result).head.sha }}" >> $GITHUB_OUTPUT
56+
57+
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} : ${{steps.get-sha.outputs.sha}} )"
58+
uses: actions/checkout@v4
59+
with:
60+
ref: ${{ steps.get-sha.outputs.sha }}
61+
62+
test3:
63+
if: github.event.comment.body == '@excalibot trigger release' && github.event.issue.pull_request
64+
runs-on: ubuntu-latest
65+
steps:
66+
- name: Get PR SHA
67+
id: sha
68+
uses: actions/github-script@v4
69+
with:
70+
result-encoding: string
71+
script: |
72+
const { owner, repo, number } = context.issue;
73+
const pr = await github.pulls.get({
74+
owner,
75+
repo,
76+
pull_number: number,
77+
});
78+
return pr.data.head.sha
79+
- uses: actions/checkout@v2
80+
with:
81+
ref: ${{ steps.sha.outputs.result }}
82+
83+
test4:
84+
if: github.event.issue.pull_request && contains(github.event.comment.body, '!bench_parser')
85+
runs-on: ubuntu-latest
86+
steps:
87+
- name: Get PR SHA
88+
id: sha
89+
uses: actions/github-script@v6
90+
with:
91+
result-encoding: string
92+
script: |
93+
const response = await github.request(context.payload.issue.pull_request.url);
94+
return response.data.head.sha;
95+
- name: Checkout PR Branch
96+
uses: actions/checkout@v3
97+
with:
98+
ref: ${{ steps.sha.outputs.result }}
99+
100+
test5:
101+
runs-on: ubuntu-20.04
102+
steps:
103+
- id: request
104+
uses: octokit/[email protected]
105+
with:
106+
route: ${{ github.event.issue.pull_request.url }}
107+
env:
108+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
109+
- name: Checkout PR Branch
110+
uses: actions/checkout@v3
111+
with:
112+
token: ${{ secrets.GITHUB_TOKEN }}
113+
repository: ${{fromJson(steps.request.outputs.data).head.repo.full_name}}
114+
ref: ${{fromJson(steps.request.outputs.data).head.ref}}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
| .github/workflows/auto_ci.yml:93:9:96:6 | Uses Step | Unpinned 3rd party Action 'Python CI' step $@ uses 'codecov/codecov-action' with ref 'v3', not a pinned commit hash | .github/workflows/auto_ci.yml:93:9:96:6 | Uses Step | Uses Step |
44
| .github/workflows/auto_ci.yml:108:9:119:6 | Uses Step: create_pr | Unpinned 3rd party Action 'Python CI' step $@ uses 'peter-evans/create-pull-request' with ref 'v5', not a pinned commit hash | .github/workflows/auto_ci.yml:108:9:119:6 | Uses Step: create_pr | Uses Step: create_pr |
55
| .github/workflows/auto_ci.yml:125:9:133:6 | Uses Step | Unpinned 3rd party Action 'Python CI' step $@ uses 'thollander/actions-comment-pull-request' with ref 'v2', not a pinned commit hash | .github/workflows/auto_ci.yml:125:9:133:6 | Uses Step | Uses Step |
6+
| .github/workflows/issue_comment_3rd_party_action.yml:12:9:17:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:12:9:17:6 | Uses Step: comment-branch | Uses Step: comment-branch |
7+
| .github/workflows/issue_comment_3rd_party_action.yml:26:9:31:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:26:9:31:6 | Uses Step: comment-branch | Uses Step: comment-branch |
8+
| .github/workflows/issue_comment_3rd_party_action.yml:40:9:46:6 | Uses Step: refs | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'eficode/resolve-pr-refs' with ref 'main', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:40:9:46:6 | Uses Step: refs | Uses Step: refs |
9+
| .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | Uses Step: fetch_issue |
10+
| .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | Uses Step: fetch_pr |
11+
| .github/workflows/issue_comment_octokit.yml:103:9:109:6 | Uses Step: request | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.0.2', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:103:9:109:6 | Uses Step: request | Uses Step: request |
612
| .github/workflows/label_trusted_checkout.yml:20:7:24:4 | Uses Step | Unpinned 3rd party Action 'label_trusted_checkout.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/label_trusted_checkout.yml:20:7:24:4 | Uses Step | Uses Step |
713
| .github/workflows/label_trusted_checkout.yml:24:7:27:21 | Uses Step | Unpinned 3rd party Action 'label_trusted_checkout.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/label_trusted_checkout.yml:24:7:27:21 | Uses Step | Uses Step |
814
| .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'v1', not a pinned commit hash | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Uses Step |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
| .github/workflows/auto_ci.yml:20:9:27:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
22
| .github/workflows/auto_ci.yml:67:9:74:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
33
| .github/workflows/gitcheckout.yml:10:11:18:8 | Run Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
4+
| .github/workflows/issue_comment_3rd_party_action.yml:17:9:23:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
5+
| .github/workflows/issue_comment_3rd_party_action.yml:31:9:37:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
6+
| .github/workflows/issue_comment_3rd_party_action.yml:46:9:50:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
7+
| .github/workflows/issue_comment_3rd_party_action.yml:50:9:53:25 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
8+
| .github/workflows/issue_comment_direct.yml:12:9:16:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
9+
| .github/workflows/issue_comment_direct.yml:20:9:24:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
10+
| .github/workflows/issue_comment_direct.yml:28:9:32:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
11+
| .github/workflows/issue_comment_direct.yml:35:9:40:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
12+
| .github/workflows/issue_comment_direct.yml:43:9:46:126 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
413
| .github/workflows/untrusted_checkout.yml:9:7:13:4 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

0 commit comments

Comments
 (0)