Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,57 @@ class GhSHACheckout extends SHACheckoutStep instanceof Run {

override string getPath() { result = this.(Run).getWorkingDirectory() }
}

private predicate isRunCheckoutReference(
PRHeadCheckoutStep checkout, Expression reference, string variable
) {
checkout instanceof Run and
reference = checkout.(Run).getInScopeEnvVarExpr(variable) and
(
checkout instanceof SHACheckoutStep and containsHeadSHA(reference.getExpression())
or
checkout instanceof MutableRefCheckoutStep and
(
containsHeadRef(reference.getExpression()) or
containsPullRequestNumber(reference.getExpression())
)
) and
exists(string command |
checkout.(Run).getScript().getACommand() = command and
exists(command.regexpFind(variable, _, _))
)
}

/** Gets the expression that controls the untrusted checkout, if one can be identified. */
AstNode getCheckoutReference(PRHeadCheckoutStep checkout) {
exists(UsesStep uses |
checkout = uses and
(
result = uses.getArgumentExpr("ref")
or
not exists(uses.getArgumentExpr("ref")) and result = uses.getArgumentExpr("repository")
)
)
or
isRunCheckoutReference(checkout, result, _)
or
checkout instanceof Run and
result = checkout and
not isRunCheckoutReference(checkout, _, _)
}

/** Gets a display label for the expression that controls the untrusted checkout. */
string getCheckoutReferenceText(AstNode reference) {
result = reference.(Expression).toString()
or
not reference instanceof Expression and result = "the checkout command"
}

/** Adds checkout-reference provenance before the checkout step in path queries. */
predicate checkoutReferenceEdge(AstNode predecessor, AstNode successor) {
exists(PRHeadCheckoutStep checkout |
predecessor = getCheckoutReference(checkout) and
successor = checkout and
not predecessor = successor
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,34 @@ import codeql.actions.security.CachePoisoningQuery
import codeql.actions.security.PoisonableSteps
import codeql.actions.security.ControlChecks

query predicate edges(Step a, Step b) { a.getNextStep() = b }
query predicate edges(AstNode predecessor, AstNode successor) {
exists(Step previous, Step next |
predecessor = previous and
successor = next and
previous.getNextStep() = next
)
or
checkoutReferenceEdge(predecessor, successor)
}

from LocalJob job, Event event, Step source, Step step, string message, string path
from
LocalJob job, Event event, Step source, Step step, string message, string path,
AstNode untrustedInput, string untrustedInputText
where
// the job checkouts untrusted code from a pull request or downloads an untrusted artifact
job.getAStep() = source and
(
source instanceof PRHeadCheckoutStep and
message = "due to privilege checkout of untrusted code." and
path = source.(PRHeadCheckoutStep).getPath()
message = "due to privilege checkout of untrusted code from" and
path = source.(PRHeadCheckoutStep).getPath() and
untrustedInput = getCheckoutReference(source) and
untrustedInputText = getCheckoutReferenceText(untrustedInput)
or
source instanceof UntrustedArtifactDownloadStep and
message = "due to downloading an untrusted artifact." and
path = source.(UntrustedArtifactDownloadStep).getPath()
message = "due to downloading" and
path = source.(UntrustedArtifactDownloadStep).getPath() and
untrustedInput = source and
untrustedInputText = "an untrusted artifact"
) and
// the checkout/download is not controlled by an access check
not exists(ControlCheck check |
Expand All @@ -57,6 +71,6 @@ where
step instanceof PoisonableStep and
// excluding privileged workflows since they can be exploited in easier circumstances
not job.isPrivileged()
select step, source, step,
"Potential cache poisoning in the context of the default branch " + message + " ($@).", event,
event.getName()
select step, untrustedInput, step,
"Potential cache poisoning in the context of the default branch " + message + " $@. ($@).",
untrustedInput, untrustedInputText, event, event.getName()
22 changes: 17 additions & 5 deletions actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,22 @@ import codeql.actions.security.UntrustedCheckoutQuery
import codeql.actions.security.PoisonableSteps
import codeql.actions.security.ControlChecks

query predicate edges(Step a, Step b) { a.getNextStep() = b }
query predicate edges(AstNode predecessor, AstNode successor) {
exists(Step previous, Step next |
predecessor = previous and
successor = next and
previous.getNextStep() = next
)
or
checkoutReferenceEdge(predecessor, successor)
}

from PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event
from
PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event, AstNode checkoutReference,
string checkoutReferenceText
where
checkoutReference = getCheckoutReference(checkout) and
checkoutReferenceText = getCheckoutReferenceText(checkoutReference) and
// the checkout is followed by a known poisonable step
checkout.getAFollowingStep() = poisonable and
(
Expand Down Expand Up @@ -51,6 +63,6 @@ where
event.getName() = checkoutTriggers() and
not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) and
not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout"))
select checkout, checkout, poisonable,
"Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@).",
event, event.getName()
select checkout, checkoutReference, poisonable,
"Checkout of untrusted code from $@ in a privileged workflow with later potential execution (event trigger: $@).",
checkoutReference, checkoutReferenceText, event, event.getName()
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: fix
---
* The `actions/cache-poisoning/poisonable-step` and `actions/untrusted-checkout/critical` queries now start paths at the expressions that control untrusted checkouts and link their alert messages to those expressions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
on: workflow_dispatch

jobs:
cache:
permissions: {}
runs-on: ubuntu-latest
steps:
- id: pr
env:
HEAD_SHA: ${{ github.event.inputs.head_sha }}
run: |
jq -cn --arg sha "$HEAD_SHA" '{head: {sha: $sha}}' |
sed 's/^/json=/' >> "$GITHUB_OUTPUT"
- env:
HEAD_SHA: ${{ fromJSON(steps.pr.outputs.json).head.sha }}
run: |
git fetch origin "$HEAD_SHA"
git checkout "$HEAD_SHA"
- run: npm install
- uses: actions/cache@v4
with:
path: .npm
key: workflow-dispatch
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
edges
| .github/workflows/cache_write_capable_workflow_dispatch.yml:8:9:14:6 | Run Step: pr | .github/workflows/cache_write_capable_workflow_dispatch.yml:14:9:19:6 | Run Step |
| .github/workflows/cache_write_capable_workflow_dispatch.yml:14:9:19:6 | Run Step | .github/workflows/cache_write_capable_workflow_dispatch.yml:19:9:20:6 | Run Step |
| .github/workflows/cache_write_capable_workflow_dispatch.yml:19:9:20:6 | Run Step | .github/workflows/cache_write_capable_workflow_dispatch.yml:20:9:23:33 | Uses Step |
| .github/workflows/code_injection2.yml:12:9:16:6 | Uses Step: modified_files | .github/workflows/code_injection2.yml:16:9:16:71 | Run Step |
| .github/workflows/direct_cache1.yml:10:9:13:6 | Uses Step: comment-branch | .github/workflows/direct_cache1.yml:13:9:18:6 | Uses Step |
| .github/workflows/direct_cache1.yml:13:9:18:6 | Uses Step | .github/workflows/direct_cache1.yml:18:9:22:6 | Uses Step |
Expand Down
Loading
Loading