Skip to content

Commit 6df70d1

Browse files
author
Alvaro Muñoz
committed
Do not consider priv events if runtime data is available
1 parent 4e94c42 commit 6df70d1

File tree

3 files changed

+58
-5
lines changed

3 files changed

+58
-5
lines changed

ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,14 @@ class JobImpl extends AstNodeImpl, TJobNode {
853853
this.getEnclosingWorkflow().getPermissions().getAPermission().matches("%write")
854854
}
855855

856+
private predicate hasRuntimeData() {
857+
exists(string path, string trigger, string name, string secrets_source, string perms |
858+
workflowDataModel(path, trigger, name, secrets_source, perms, _) and
859+
path.trim() = this.getLocation().getFile().getRelativePath() and
860+
name.trim().matches(this.getId() + "%")
861+
)
862+
}
863+
856864
private predicate hasRuntimeWritePermissions() {
857865
// the effective runtime permissions have write access
858866
exists(string path, string trigger, string name, string secrets_source, string perms |
@@ -885,15 +893,18 @@ class JobImpl extends AstNodeImpl, TJobNode {
885893
/** Holds if the action is privileged and externally triggerable. */
886894
predicate isPrivilegedExternallyTriggerable() {
887895
exists(EventImpl e |
888-
// job is triggereable by an external user
889896
this.getATriggerEvent() = e and
897+
// job is triggereable by an external user
890898
e.isExternallyTriggerable() and
891-
// job is privileged (write access or access to secrets)
899+
// no matter if `pull_request` is granted write permissions or access to secrets
900+
// when the job is triggered by a `pull_request` event from a fork, they will get revoked
901+
not e.getName() = "pull_request" and
892902
(
893-
this.isPrivileged() and
894-
not e.getName() = "pull_request"
903+
// job is privileged (write access or access to secrets)
904+
this.isPrivileged()
895905
or
896-
not this.isPrivileged() and
906+
// the trigger event is __normally__ privileged and we have no runtime data to prove otherwise
907+
not this.hasRuntimeData() and
897908
e.isPrivileged()
898909
)
899910
)
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: "Test"
2+
permissions:
3+
actions: none
4+
checks: none
5+
contents: read
6+
deployments: none
7+
id-token: none
8+
issues: none
9+
discussions: none
10+
packages: none
11+
pages: none
12+
pull-requests: read
13+
repository-projects: none
14+
security-events: none
15+
statuses: none
16+
on:
17+
pull_request_target:
18+
types:
19+
- opened
20+
- edited
21+
- synchronize
22+
23+
jobs:
24+
main:
25+
name: Test Pull Request
26+
runs-on: ubuntu-latest
27+
steps:
28+
- name: Checkout
29+
uses: actions/checkout@v4
30+
with:
31+
ref: ${{ github.event.pull_request.head.ref }}
32+
repository: ${{ github.event.pull_request.head.repo.full_name }}
33+
- run: npm install
34+
working-directory: scripts/github-actions/semantic-pull-request/
35+
- name: Lint PR Title
36+
if: github.event_name == 'pull_request_target'
37+
uses: actions/github-script@v7
38+
with:
39+
script: |
40+
const verifyPullRequest = require('./scripts/github-actions/semantic-pull-request')
41+
await verifyPullRequest({ context, core, github })

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44
| .github/workflows/level0.yml:99:9:103:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
55
| .github/workflows/level0.yml:125:9:129:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
66
| .github/workflows/mend.yml:22:9:29:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
7+
| .github/workflows/test3.yml:28:9:33:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
78
| .github/workflows/untrusted_checkout.yml:10:9:13:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
89
| .github/workflows/untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

0 commit comments

Comments
 (0)