hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refactor untrusted checkout queries

Browse Source
This commit is contained in:
Alvaro Muñoz
2024-05-04 23:27:26 +02:00
parent 9843f375ee
commit 16c77cbe25
19 changed files with 383 additions and 244 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll
View File

@@ -3,6 +3,7 @@ private import codeql.actions.TaintTracking
import codeql.actions.DataFlow
private import codeql.actions.dataflow.ExternalFlow
import codeql.actions.dataflow.FlowSources
import codeql.actions.security.PoisonableSteps
string unzipRegexp() { result = ".*(unzip|tar)\\s+.*" }
@@ -228,81 +229,19 @@ class DirectArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run {
}
}
abstract class PoisonableStep extends Step { }
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/rules/untrusted_checkout_exec.rego#L16
private string dangerousActions() {
result =
["pre-commit/action", "oxsecurity/megalinter", "bridgecrewio/checkov-action", "ruby/setup-ruby"]
}
class DangerousActionUsesStep extends PoisonableStep, UsesStep {
DangerousActionUsesStep() {
exists(UntrustedArtifactDownloadStep step |
step.getAFollowingStep() = this and
this.getCallee() = dangerousActions()
)
}
}
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/rules/untrusted_checkout_exec.rego#L23
private string dangerousCommands() {
result =
[
"npm install", "npm run ", "yarn ", "npm ci(\\b|$)", "make ", "terraform plan",
"terraform apply", "gomplate ", "pre-commit run", "pre-commit install", "go generate",
"msbuild ", "mvn ", "./mvnw ", "gradle ", "./gradlew ", "bundle install", "bundle exec ",
"^ant ", "mkdocs build", "pytest"
]
}
class BuildRunStep extends PoisonableStep, Run {
BuildRunStep() {
exists(UntrustedArtifactDownloadStep step |
step.getAFollowingStep() = this and
exists(
this.getScript().splitAt("\n").trim().regexpFind("([^a-z]|^)" + dangerousCommands(), _, _)
)
)
}
}
class LocalCommandExecutionRunStep extends PoisonableStep, Run {
LocalCommandExecutionRunStep() {
exists(UntrustedArtifactDownloadStep step |
step.getAFollowingStep() = this and
// Heuristic:
// Run step with a command starting with `./xxxx`, `sh xxxx`, ...
exists(
this.getScript()
.splitAt("\n")
.trim()
.regexpFind("([^a-z]|^)(./|(ba|z|fi)?sh\\s+)" + step.getPath(), _, _)
)
)
}
}
class EnvVarInjectionRunStep extends PoisonableStep, Run {
EnvVarInjectionRunStep() {
exists(UntrustedArtifactDownloadStep step, string value |
step.getAFollowingStep() = this and
// Heuristic:
// Run step with env var definition based on file content.
// eg: `echo "sha=$(cat test-results/sha-number)" >> $GITHUB_ENV`
// eg: `echo "sha=$(<test-results/sha-number)" >> $GITHUB_ENV`
Utils::writeToGitHubEnv(this, _, value) and
// TODO: add support for other commands like `<`, `jq`, ...
value.regexpMatch(["\\$\\(", "`"] + ["ls\\s+", "cat\\s+", "<"] + ".*" + ["`", "\\)"])
)
}
}
class ArtifactPoisoningSink extends DataFlow::Node {
ArtifactPoisoningSink() {
exists(PoisonableStep step |
step.(Run).getScriptScalar() = this.asExpr() or
step.(UsesStep) = this.asExpr()
exists(UntrustedArtifactDownloadStep download, PoisonableStep poisonable |
download.getAFollowingStep() = poisonable and
(
poisonable.(Run).getScriptScalar() = this.asExpr()
or
poisonable.(UsesStep) = this.asExpr()
) and
(
not poisonable instanceof LocalCommandExecutionRunStep or
poisonable.(LocalCommandExecutionRunStep).getCommand().matches(download.getPath() + "%")
)
)
}
}

64
ql/lib/codeql/actions/security/PoisonableSteps.qll Normal file
View File

@@ -0,0 +1,64 @@
import actions
abstract class PoisonableStep extends Step { }
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/rules/untrusted_checkout_exec.rego#L16
private string dangerousActions() {
result =
["pre-commit/action", "oxsecurity/megalinter", "bridgecrewio/checkov-action", "ruby/setup-ruby"]
}
class DangerousActionUsesStep extends PoisonableStep, UsesStep {
DangerousActionUsesStep() { this.getCallee() = dangerousActions() }
}
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/rules/untrusted_checkout_exec.rego#L23
private string dangerousCommands() {
result =
[
"npm install", "npm run ", "yarn ", "npm ci(\\b|$)", "make ", "terraform plan",
"terraform apply", "gomplate ", "pre-commit run", "pre-commit install", "go generate",
"msbuild ", "mvn ", "./mvnw ", "gradle ", "./gradlew ", "bundle install", "bundle exec ",
"^ant ", "mkdocs build", "pytest"
]
}
class BuildRunStep extends PoisonableStep, Run {
BuildRunStep() {
exists(
this.getScript().splitAt("\n").trim().regexpFind("([^a-z]|^)" + dangerousCommands(), _, _)
)
}
}
class LocalCommandExecutionRunStep extends PoisonableStep, Run {
string cmd;
LocalCommandExecutionRunStep() {
// Heuristic:
// Run step with a command starting with `./xxxx`, `sh xxxx`, ...
exists(string line | line = this.getScript().splitAt("\n").trim() |
// ./xxxx
cmd = line.regexpCapture("(^|\\s+)\\.\\/(.*)", 2)
or
// sh xxxx
cmd = line.regexpCapture("(^|\\s+)(ba|z|fi)?sh\\s+(.*)", 3)
)
}
string getCommand() { result = cmd }
}
class EnvVarInjectionRunStep extends PoisonableStep, Run {
EnvVarInjectionRunStep() {
exists(string value |
// Heuristic:
// Run step with env var definition based on file content.
// eg: `echo "sha=$(cat test-results/sha-number)" >> $GITHUB_ENV`
// eg: `echo "sha=$(<test-results/sha-number)" >> $GITHUB_ENV`
Utils::writeToGitHubEnv(this, _, value) and
// TODO: add support for other commands like `<`, `jq`, ...
value.regexpMatch(["\\$\\(", "`"] + ["ls\\s+", "cat\\s+", "<"] + ".*" + ["`", "\\)"])
)
}
}

229
ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll Normal file
View File

@@ -0,0 +1,229 @@
import actions
import codeql.actions.DataFlow
bindingset[s]
predicate containsPullRequestNumber(string s) {
exists(
Utils::normalizeExpr(s)
.regexpFind([
"\\bgithub\\.event\\.number\\b", "\\bgithub\\.event\\.issue\\.number\\b",
"\\bgithub\\.event\\.pull_request\\.id\\b",
"\\bgithub\\.event\\.pull_request\\.number\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.number\\b",
// heuristics
"\\bpr_number\\b", "\\bpr_id\\b"
], _, _)
)
}
bindingset[s]
predicate containsHeadSHA(string s) {
exists(
Utils::normalizeExpr(s)
.regexpFind([
"\\bgithub\\.event\\.pull_request\\.head\\.sha\\b",
"\\bgithub\\.event\\.pull_request\\.merge_commit_sha\\b",
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.id\\b",
"\\bgithub\\.event\\.workflow_run\\.head_sha\\b",
"\\bgithub\\.event\\.check_suite\\.after\\b",
"\\bgithub\\.event\\.check_suite\\.head_commit\\.id\\b",
"\\bgithub\\.event\\.check_suite\\.head_sha\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.after\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.head_commit\\.id\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.head_sha\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_run\\.head_sha\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
// heuristics
"\\bhead\\.sha\\b", "\\bhead_sha\\b", "\\bpr_head_sha\\b"
], _, _)
)
}
bindingset[s]
predicate containsHeadRef(string s) {
exists(
Utils::normalizeExpr(s)
.regexpFind([
"\\bgithub\\.event\\.pull_request\\.head\\.ref\\b", "\\bgithub\\.head_ref\\b",
"\\bgithub\\.event\\.workflow_run\\.head_branch\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
// heuristics
"\\bhead\\.ref\\b", "\\bhead_ref\\b", "\\bpr_head_ref\\b",
// env vars
"\\benv\\.GITHUB_HEAD_REF\\b",
], _, _)
)
}
/** Checkout of a Pull Request HEAD ref */
abstract class PRHeadCheckoutStep extends Step { }
/** Checkout of a Pull Request HEAD ref using actions/checkout action */
class ActionsMutableRefCheckout extends PRHeadCheckoutStep instanceof UsesStep {
ActionsMutableRefCheckout() {
this.getCallee() = "actions/checkout" and
(
// ref argument contains the PR id/number or head ref/sha
exists(Expression e |
(
containsHeadRef(e.getExpression()) or
containsPullRequestNumber(e.getExpression())
) and
DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))
)
or
// 3rd party actions returning the PR head sha/ref
exists(UsesStep step |
step.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and
// TODO: This should be read step of the head_sha or head_ref output vars
this.getArgument("ref").regexpMatch(".*head_ref.*") and
DataFlow::hasLocalFlowExpr(step, this.getArgumentExpr("ref"))
)
or
// heuristic base on the step id and field name
exists(StepsExpression e |
this.getArgumentExpr("ref") = e and
(
e.getStepId().matches(["%ref%", "%branch%"]) or
e.getFieldName().matches(["%ref%", "%branch%"])
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using actions/checkout action */
class ActionsSHACheckout extends PRHeadCheckoutStep instanceof UsesStep {
ActionsSHACheckout() {
this.getCallee() = "actions/checkout" and
(
// ref argument contains the PR id/number or head ref/sha
exists(Expression e |
containsHeadSHA(e.getExpression()) and
DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))
)
or
// 3rd party actions returning the PR head sha/ref
exists(UsesStep step |
step.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and
this.getArgument("ref").regexpMatch(".*head_sha.*") and
DataFlow::hasLocalFlowExpr(step, this.getArgumentExpr("ref"))
)
or
// heuristic base on the step id and field name
exists(StepsExpression e |
this.getArgumentExpr("ref") = e and
(
e.getStepId().matches(["%sha%", "%commit%"]) or
e.getFieldName().matches(["%sha%", "%commit%"])
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using git within a Run step */
class GitMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
GitMutableRefCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*git\\s+(fetch|pull).*") and
(
(containsHeadRef(line) or containsPullRequestNumber(line))
or
exists(string varname, string expr |
expr = this.getInScopeEnvVarExpr(varname).getExpression() and
(
containsHeadRef(expr) or
containsPullRequestNumber(expr)
) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using git within a Run step */
class GitSHACheckout extends PRHeadCheckoutStep instanceof Run {
GitSHACheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*git\\s+(fetch|pull).*") and
(
containsHeadSHA(line)
or
exists(string varname, string expr |
expr = this.getInScopeEnvVarExpr(varname).getExpression() and
containsHeadSHA(expr) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
class GhMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
GhMutableRefCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*gh\\s+pr\\s+checkout.*") and
(
(containsHeadRef(line) or containsPullRequestNumber(line))
or
exists(string varname |
(
containsHeadRef(this.getInScopeEnvVarExpr(varname).getExpression()) or
containsPullRequestNumber(this.getInScopeEnvVarExpr(varname).getExpression())
) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
class GhSHACheckout extends PRHeadCheckoutStep instanceof Run {
GhSHACheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*gh\\s+pr\\s+checkout.*") and
(
containsHeadSHA(line)
or
exists(string varname |
containsHeadSHA(this.getInScopeEnvVarExpr(varname).getExpression()) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
/** An If node that contains an actor, user or label check */
class ControlCheck extends If {
ControlCheck() {
exists(
Utils::normalizeExpr(this.getCondition())
.regexpFind([
"\\bgithub\\.actor\\b", // actor
"\\bgithub\\.triggering_actor\\b", // actor
"\\bgithub\\.event\\.comment\\.user\\.login\\b", //user
"\\bgithub\\.event\\.pull_request\\.user\\.login\\b", //user
"\\bgithub\\.event\\.pull_request\\.labels\\b", // label
"\\bgithub\\.event\\.label\\.name\\b" // label
], _, _)
)
}
}

2
ql/lib/qlpack.yml
View File

@@ -2,7 +2,7 @@
library: true
warnOnImplicitThis: true
name: githubsecuritylab/actions-all
version: 0.0.19
version: 0.0.20
dependencies:
codeql/util: ^0.2.0
codeql/yaml: ^0.1.2

150
ql/src/Security/CWE-829/UntrustedCheckout.ql
View File

@@ -1,150 +0,0 @@
/**
* @name Checkout of untrusted code in trusted context
* @description Priveleged workflows have read/write access to the base repository and access to secrets.
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
* that is able to push to the base repository and to access secrets.
* @kind problem
* @problem.severity warning
* @precision medium
* @security-severity 9.3
* @id actions/untrusted-checkout
* @tags actions
* security
* external/cwe/cwe-829
*/
import actions
import codeql.actions.DataFlow
/** An If node that contains an actor, user or label check */
class ControlCheck extends If {
ControlCheck() {
exists(
Utils::normalizeExpr(this.getCondition())
.regexpFind([
"\\bgithub\\.actor\\b", // actor
"\\bgithub\\.triggering_actor\\b", // actor
"\\bgithub\\.event\\.comment\\.user\\.login\\b", //user
"\\bgithub\\.event\\.pull_request\\.user\\.login\\b", //user
"\\bgithub\\.event\\.pull_request\\.labels\\b", // label
"\\bgithub\\.event\\.label\\.name\\b" // label
], _, _)
)
}
}
bindingset[s]
predicate containsHeadRef(string s) {
exists(
Utils::normalizeExpr(s)
.regexpFind([
"\\bgithub\\.event\\.number\\b", // The pull request number.
"\\bgithub\\.event\\.issue\\.number\\b", // The pull request number on issue_comment.
"\\bgithub\\.event\\.pull_request\\.head\\.ref\\b", // The ref name of head.
"\\bgithub\\.event\\.pull_request\\.head\\.sha\\b", // The commit SHA of head.
"\\bgithub\\.event\\.pull_request\\.id\\b", // The pull request ID.
"\\bgithub\\.event\\.pull_request\\.number\\b", // The pull request number.
"\\bgithub\\.event\\.pull_request\\.merge_commit_sha\\b", // The SHA of the merge commit.
"\\bgithub\\.head_ref\\b", // The head_ref or source branch of the pull request in a workflow run.
"\\bgithub\\.event\\.workflow_run\\.head_branch\\b", // The branch of the head commit.
"\\bgithub\\.event\\.workflow_run\\.head_commit\\.id\\b", // The SHA of the head commit.
"\\bgithub\\.event\\.workflow_run\\.head_sha\\b", // The SHA of the head commit.
"\\benv\\.GITHUB_HEAD_REF\\b", "\\bgithub\\.event\\.check_suite\\.after\\b",
"\\bgithub\\.event\\.check_suite\\.head_sha\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.after\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.head_sha\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.number\\b",
"\\bgithub\\.event\\.check_run\\.head_sha\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.id\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.number\\b",
// heuristics
"\\bhead\\.sha\\b", "\\bhead\\.ref\\b", "\\bpr_number\\b", "\\bpr_head_sha\\b"
], _, _)
)
}
/** Checkout of a Pull Request HEAD ref */
abstract class PRHeadCheckoutStep extends Step { }
/** Checkout of a Pull Request HEAD ref using actions/checkout action */
class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep {
ActionsCheckout() {
this.getCallee() = "actions/checkout" and
(
// ref argument contains the head ref
exists(Expression e |
containsHeadRef(e.getExpression()) and
DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))
)
or
// 3rd party actions returning the PR head sha/ref
exists(UsesStep head |
head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and
DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref"))
)
or
// heuristic base on the step id and field name
exists(StepsExpression e |
this.getArgumentExpr("ref") = e and
(
e.getStepId().matches(["%sha%", "%head%", "branch"]) or
e.getFieldName().matches(["%sha%", "%head%", "branch"])
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using git within a Run step */
class GitCheckout extends PRHeadCheckoutStep instanceof Run {
GitCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*git\\s+fetch.*") and
(
containsHeadRef(line)
or
exists(string varname |
containsHeadRef(this.getInScopeEnvVarExpr(varname).getExpression()) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
class GhCheckout extends PRHeadCheckoutStep instanceof Run {
GhCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
line.regexpMatch(".*gh\\s+pr\\s+checkout.*") and
(
containsHeadRef(line)
or
exists(string varname |
containsHeadRef(this.getInScopeEnvVarExpr(varname).getExpression()) and
exists(line.regexpFind(varname, _, _))
)
)
)
}
}
from Workflow w, PRHeadCheckoutStep checkout
where
w.isPrivileged() and
w.getAJob().(LocalJob).getAStep() = checkout and
not exists(ControlCheck check |
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check
)
select checkout, "Potential unsafe checkout of untrusted pull request on privileged workflow."

0
ql/src/Security/CWE-829/UntrustedCheckout.md → ql/src/Security/CWE-829/UntrustedCheckoutError.md
View File

28
ql/src/Security/CWE-829/UntrustedCheckoutError.ql Normal file
View File

@@ -0,0 +1,28 @@
/**
* @name Checkout of untrusted code in trusted context
* @description Priveleged workflows have read/write access to the base repository and access to secrets.
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
* that is able to push to the base repository and to access secrets.
* @kind problem
* @problem.severity error
* @precision high
* @security-severity 9.3
* @id actions/untrusted-checkout
* @tags actions
* security
* external/cwe/cwe-829
*/
import actions
import codeql.actions.security.UntrustedCheckoutQuery
import codeql.actions.security.PoisonableSteps
from Workflow w, PRHeadCheckoutStep checkout
where
w.isPrivileged() and
w.getAJob().(LocalJob).getAStep() = checkout and
checkout.getAFollowingStep() instanceof PoisonableStep and
not exists(ControlCheck check |
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check
)
select checkout, "Potential unsafe checkout of untrusted pull request on privileged workflow."

0
ql/src/Security/CWE-829/UntrustedCheckoutWarning.md Normal file
View File

28
ql/src/Security/CWE-829/UntrustedCheckoutWarning.ql Normal file
View File

@@ -0,0 +1,28 @@
/**
* @name Checkout of untrusted code in trusted context
* @description Priveleged workflows have read/write access to the base repository and access to secrets.
* By explicitly checking out and running the build script from a fork the untrusted code is running in an environment
* that is able to push to the base repository and to access secrets.
* @kind problem
* @problem.severity warning
* @precision medium
* @security-severity 5.3
* @id actions/untrusted-checkout
* @tags actions
* security
* external/cwe/cwe-829
*/
import actions
import codeql.actions.security.UntrustedCheckoutQuery
import codeql.actions.security.PoisonableSteps
from Workflow w, PRHeadCheckoutStep checkout
where
w.isPrivileged() and
w.getAJob().(LocalJob).getAStep() = checkout and
not checkout.getAFollowingStep() instanceof PoisonableStep and
not exists(ControlCheck check |
checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check
)
select checkout, "Potential unsafe checkout of untrusted pull request on privileged workflow."

2
ql/src/qlpack.yml
View File

@@ -1,7 +1,7 @@
---
library: false
name: githubsecuritylab/actions-queries
version: 0.0.19
version: 0.0.20
groups:
- actions
- queries

2
ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning21.yml vendored
View File

@@ -17,7 +17,7 @@ jobs:
path: foo
- name: Run command
run: |
./foo/cmd
sh foo/cmd

2
ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning22.yml vendored
View File

@@ -15,7 +15,7 @@ jobs:
name: artifact_name
workflow: wf.yml
- name: Run command
run: ./cmd
run: sh cmd

8
ql/test/query-tests/Security/CWE-829/ArtifactPoisoning.expected
View File

@@ -1,8 +1,8 @@
edges
| .github/workflows/artifactpoisoning11.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning11.yml:38:11:38:77 | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning12.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd |
| .github/workflows/artifactpoisoning31.yml:13:9:15:6 | Run Step | .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd |
| .github/workflows/artifactpoisoning32.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning32.yml:17:14:18:20 | ./bar/cmd\n |
| .github/workflows/artifactpoisoning33.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning33.yml:17:14:18:20 | ./bar/cmd\n |
@@ -18,9 +18,9 @@ nodes
| .github/workflows/artifactpoisoning12.yml:13:9:32:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build | semmle.label | ./x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n | semmle.label | ./foo/cmd\n |
| .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n | semmle.label | sh foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd | semmle.label | ./cmd |
| .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd | semmle.label | sh cmd |
| .github/workflows/artifactpoisoning31.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
| .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd | semmle.label | ./foo/cmd |
| .github/workflows/artifactpoisoning32.yml:13:9:16:6 | Run Step | semmle.label | Run Step |

12
ql/test/query-tests/Security/CWE-829/PrivilegedArtifactPoisoning.expected
View File

@@ -1,8 +1,8 @@
edges
| .github/workflows/artifactpoisoning11.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning11.yml:38:11:38:77 | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning12.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd |
| .github/workflows/artifactpoisoning31.yml:13:9:15:6 | Run Step | .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd |
| .github/workflows/artifactpoisoning32.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning32.yml:17:14:18:20 | ./bar/cmd\n |
| .github/workflows/artifactpoisoning33.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning33.yml:17:14:18:20 | ./bar/cmd\n |
@@ -18,9 +18,9 @@ nodes
| .github/workflows/artifactpoisoning12.yml:13:9:32:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build | semmle.label | ./x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n | semmle.label | ./foo/cmd\n |
| .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n | semmle.label | sh foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd | semmle.label | ./cmd |
| .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd | semmle.label | sh cmd |
| .github/workflows/artifactpoisoning31.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
| .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd | semmle.label | ./foo/cmd |
| .github/workflows/artifactpoisoning32.yml:13:9:16:6 | Run Step | semmle.label | Run Step |
@@ -43,8 +43,8 @@ subpaths
#select
| .github/workflows/artifactpoisoning11.yml:38:11:38:77 | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build | .github/workflows/artifactpoisoning11.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning11.yml:38:11:38:77 | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning11.yml:38:11:38:77 | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build | ./sonarcloud-data/x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build | .github/workflows/artifactpoisoning12.yml:13:9:32:6 | Uses Step | .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning12.yml:38:11:38:61 | ./x.py build -j$(nproc) --compiler gcc --skip-build | ./x.py build -j$(nproc) --compiler gcc --skip-build |
| .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n | .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning21.yml:19:14:20:20 | ./foo/cmd\n | ./foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd | .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning22.yml:18:14:18:18 | ./cmd | ./cmd |
| .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n | .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning21.yml:19:14:20:21 | sh foo/cmd\n | sh foo/cmd\n |
| .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd | .github/workflows/artifactpoisoning22.yml:13:9:17:6 | Uses Step | .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning22.yml:18:14:18:19 | sh cmd | sh cmd |
| .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd | .github/workflows/artifactpoisoning31.yml:13:9:15:6 | Run Step | .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning31.yml:19:14:19:22 | ./foo/cmd | ./foo/cmd |
| .github/workflows/artifactpoisoning32.yml:17:14:18:20 | ./bar/cmd\n | .github/workflows/artifactpoisoning32.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning32.yml:17:14:18:20 | ./bar/cmd\n | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning32.yml:17:14:18:20 | ./bar/cmd\n | ./bar/cmd\n |
| .github/workflows/artifactpoisoning33.yml:17:14:18:20 | ./bar/cmd\n | .github/workflows/artifactpoisoning33.yml:13:9:16:6 | Run Step | .github/workflows/artifactpoisoning33.yml:17:14:18:20 | ./bar/cmd\n | Potential privileged artifact poisoning in $@, which may be controlled by an external user. | .github/workflows/artifactpoisoning33.yml:17:14:18:20 | ./bar/cmd\n | ./bar/cmd\n |

1
ql/test/query-tests/Security/CWE-829/UntrustedCheckout.qlref
View File

@@ -1 +0,0 @@
Security/CWE-829/UntrustedCheckout.ql

6
ql/test/query-tests/Security/CWE-829/UntrustedCheckoutError.expected Normal file
View File

@@ -0,0 +1,6 @@
| .github/workflows/auto_ci.yml:67:9:74:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/gitcheckout.yml:10:11:18:8 | Run Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/level0.yml:99:9:103:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/level0.yml:125:9:129:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/untrusted_checkout.yml:10:9:13:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

1
ql/test/query-tests/Security/CWE-829/UntrustedCheckoutError.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE-829/UntrustedCheckoutError.ql

6
ql/test/query-tests/Security/CWE-829/UntrustedCheckout.expected → ql/test/query-tests/Security/CWE-829/UntrustedCheckoutWarning.expected
View File

@@ -1,6 +1,4 @@
| .github/workflows/auto_ci.yml:20:9:27:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/auto_ci.yml:67:9:74:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/gitcheckout.yml:10:11:18:8 | Run Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/issue_comment_3rd_party_action.yml:16:9:22:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/issue_comment_3rd_party_action.yml:30:9:36:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/issue_comment_3rd_party_action.yml:45:9:49:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
@@ -18,10 +16,6 @@
| .github/workflows/issue_comment_octokit.yml:79:9:83:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/issue_comment_octokit.yml:95:9:100:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/issue_comment_octokit.yml:109:9:114:66 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/level0.yml:99:9:103:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/level0.yml:125:9:129:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/untrusted_checkout2.yml:14:9:19:72 | Run Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/untrusted_checkout.yml:10:9:13:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/workflow_run_untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
| .github/workflows/workflow_run_untrusted_checkout.yml:16:9:18:31 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

1
ql/test/query-tests/Security/CWE-829/UntrustedCheckoutWarning.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE-829/UntrustedCheckoutWarning.ql