Actions: improve improper access control query

Closes: https://github.com/github/codeql/issues/20706
This commit is contained in:
Paolo Tranquilli
2025-11-25 08:46:10 +01:00
parent daead038ab
commit b2d9c58db9
6 changed files with 32 additions and 4 deletions

View File

@@ -14,7 +14,7 @@
import codeql.actions.security.UntrustedCheckoutQuery import codeql.actions.security.UntrustedCheckoutQuery
import codeql.actions.security.ControlChecks import codeql.actions.security.ControlChecks
from LocalJob job, LabelCheck check, MutableRefCheckoutStep checkout, Event event from LocalJob job, LabelCheck check, PRHeadCheckoutStep checkout, Event event
where where
job.isPrivileged() and job.isPrivileged() and
job.getAStep() = checkout and job.getAStep() = checkout and
@@ -25,6 +25,8 @@ where
event.getAnActivityType() = "synchronize" event.getAnActivityType() = "synchronize"
or or
not exists(job.getATriggerEvent()) not exists(job.getATriggerEvent())
or
checkout instanceof MutableRefCheckoutStep
) )
select checkout, "The checked-out code can be modified after the authorization check $@.", check, select checkout, "The checked-out code can be modified after the authorization check $@.", check,
check.toString() check.toString()

View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* The `actions/improper-access-control` query has been improved to correctly detect cases where either the check
triggers or the checkout reference are unsafe, rather than only when both applied as was done previously.

View File

@@ -16,5 +16,5 @@ jobs:
uses: actions/checkout@v3 uses: actions/checkout@v3
if: contains(github.event.pull_request.labels.*.name, 'safe to test') if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with: with:
ref: ${{ github.event.pull_request.head.ref }} ref: ${{ github.event.pull_request.head.ref }} # BAD (mutable ref)
- run: ./cmd - run: ./cmd

View File

@@ -16,5 +16,5 @@ jobs:
uses: actions/checkout@v3 uses: actions/checkout@v3
if: contains(github.event.pull_request.labels.*.name, 'safe to test') if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with: with:
ref: ${{ github.event.pull_request.head.ref }} ref: ${{ github.event.pull_request.head.sha }} # BAD (bad trigger)
- run: ./cmd - run: ./cmd

View File

@@ -0,0 +1,20 @@
name: Pull request feedback
on:
pull_request_target:
types: [ labeled ]
permissions: {}
jobs:
test:
permissions:
contents: write
pull-requests: write
runs-on: ubuntu-latest
steps:
- name: Checkout repo for OWNER TEST
uses: actions/checkout@v3
if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with:
ref: ${{ github.event.pull_request.head.sha }} # GOOD (labeled event + immutable ref)
- run: ./cmd

View File

@@ -1 +1,2 @@
| .github/workflows/test1.yml:15:7:20:4 | Uses Step | The checked-out code can be modified after the authorization check $@. | .github/workflows/test1.yml:17:11:17:75 | contain ... test') | contain ... test') | | .github/workflows/bad_checkout.yml:15:7:20:4 | Uses Step | The checked-out code can be modified after the authorization check $@. | .github/workflows/bad_checkout.yml:17:11:17:75 | contain ... test') | contain ... test') |
| .github/workflows/bad_triggers.yml:15:7:20:4 | Uses Step | The checked-out code can be modified after the authorization check $@. | .github/workflows/bad_triggers.yml:17:11:17:75 | contain ... test') | contain ... test') |