hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-28 06:36:33 +01:00
Code Issues Packages Projects Releases Wiki Activity

feat(queries): Consider untrusted checkout as a source for code injections

Browse Source
This commit is contained in:
Alvaro Muñoz
2024-07-15 21:00:28 +02:00
parent 76ded33280
commit fc39249f92
8 changed files with 79 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
ql/lib/codeql/actions/dataflow/FlowSources.qll
View File

@@ -1,4 +1,5 @@
private import codeql.actions.security.ArtifactPoisoningQuery
private import codeql.actions.security.UntrustedCheckoutQuery
private import codeql.actions.config.Config
private import codeql.actions.dataflow.ExternalFlow
@@ -112,6 +113,15 @@ private class ArtifactSource extends RemoteFlowSource {
override string getSourceType() { result = "artifact" }
}
/**
* A file from an untrusted checkout.
*/
private class CheckoutSource extends RemoteFlowSource {
CheckoutSource() { this.asExpr() instanceof PRHeadCheckoutStep }
override string getSourceType() { result = "artifact" }
}
/**
* A list of file names returned by dorny/paths-filter.
*/

37
ql/lib/codeql/actions/dataflow/FlowSteps.qll
View File

@@ -8,6 +8,7 @@ private import codeql.actions.DataFlow
private import codeql.actions.dataflow.FlowSources
private import codeql.actions.dataflow.ExternalFlow
private import codeql.actions.security.ArtifactPoisoningQuery
private import codeql.actions.security.UntrustedCheckoutQuery
/**
* A unit class for adding additional taint steps.
@@ -161,7 +162,11 @@ predicate envToEnvStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::
* echo "::set-output name=id::$foo
*/
predicate artifactToOutputStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) {
exists(Run run, UntrustedArtifactDownloadStep download, string content, string key, string value |
exists(Run run, Step artifact, string content, string key, string value |
(
artifact instanceof UntrustedArtifactDownloadStep or
artifact instanceof PRHeadCheckoutStep
) and
(
// A file is read and its content is assigned to an env var
// - run: |
@@ -185,7 +190,7 @@ predicate artifactToOutputStoreStep(DataFlow::Node pred, DataFlow::Node succ, Da
outputsPartialFileContent(value)
) and
c = any(DataFlow::FieldContent ct | ct.getName() = key) and
download.getAFollowingStep() = run and
artifact.getAFollowingStep() = run and
pred.asExpr() = run.getScriptScalar() and
succ.asExpr() = run
)
@@ -199,7 +204,11 @@ predicate artifactToOutputStoreStep(DataFlow::Node pred, DataFlow::Node succ, Da
* echo "bar=${foo}" >> "$GITHUB_ENV"
*/
predicate artifactToEnvStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) {
exists(Run run, string content, string key, string value, UntrustedArtifactDownloadStep download |
exists(Run run, string content, string key, string value, Step artifact |
(
artifact instanceof UntrustedArtifactDownloadStep or
artifact instanceof PRHeadCheckoutStep
) and
(
// A file is read and its content is assigned to an env var
// - run: |
@@ -223,7 +232,7 @@ predicate artifactToEnvStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataF
outputsPartialFileContent(value)
) and
c = any(DataFlow::FieldContent ct | ct.getName() = key) and
download.getAFollowingStep() = run and
artifact.getAFollowingStep() = run and
pred.asExpr() = run.getScriptScalar() and
// we store the taint on the enclosing job since there may not be an implicit env attribute
succ.asExpr() = run.getEnclosingJob()
@@ -234,10 +243,14 @@ predicate artifactToEnvStoreStep(DataFlow::Node pred, DataFlow::Node succ, DataF
* A download artifact step followed by a step that may use downloaded artifacts.
*/
predicate artifactDownloadToRunStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(UntrustedArtifactDownloadStep download, Run run |
pred.asExpr() = download and
exists(Step artifact, Run run |
(
artifact instanceof UntrustedArtifactDownloadStep or
artifact instanceof PRHeadCheckoutStep
) and
pred.asExpr() = artifact and
succ.asExpr() = run.getScriptScalar() and
download.getAFollowingStep() = run
artifact.getAFollowingStep() = run
)
}
@@ -245,11 +258,15 @@ predicate artifactDownloadToRunStep(DataFlow::Node pred, DataFlow::Node succ) {
* A download artifact step followed by a envvar-injection uses step .
*/
predicate artifactDownloadToUsesStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(UntrustedArtifactDownloadStep download, Uses uses |
exists(Step artifact, Uses uses |
(
artifact instanceof UntrustedArtifactDownloadStep or
artifact instanceof PRHeadCheckoutStep
) and
madSink(succ, "envvar-injection") and
pred.asExpr() = download and
pred.asExpr() = artifact and
succ.asExpr() = uses and
download.getAFollowingStep() = uses
artifact.getAFollowingStep() = uses
)
}

2
ql/lib/qlpack.yml
View File

@@ -2,7 +2,7 @@
library: true
warnOnImplicitThis: true
name: github/actions-all
version: 0.1.21
version: 0.1.22
dependencies:
codeql/util: ^1.0.1
codeql/yaml: ^1.0.1

16
ql/src/Security/CWE-349/CachePoisoning.ql
View File

@@ -12,6 +12,7 @@
*/
import actions
import codeql.actions.security.ArtifactPoisoningQuery
import codeql.actions.security.UntrustedCheckoutQuery
import codeql.actions.security.CachePoisoningQuery
import codeql.actions.security.PoisonableSteps
@@ -19,13 +20,17 @@ import codeql.actions.security.ControlChecks
query predicate edges(Step a, Step b) { a.getNextStep() = b }
from LocalJob j, Event e, PRHeadCheckoutStep checkout, Step s
from LocalJob j, Event e, Step artifact, Step s
where
(
artifact instanceof PRHeadCheckoutStep or
artifact instanceof UntrustedArtifactDownloadStep
) and
j.getATriggerEvent() = e and
// job can be triggered by an external user
e.isExternallyTriggerable() and
// the checkout is not controlled by an access check
not exists(ControlCheck check | check.protects(checkout, j.getATriggerEvent())) and
not exists(ControlCheck check | check.protects(artifact, j.getATriggerEvent())) and
(
// the workflow runs in the context of the default branch
runsOnDefaultBranch(e)
@@ -38,8 +43,7 @@ where
)
) and
// the job checkouts untrusted code from a pull request
// TODO: Consider adding artifact downloads as a potential source of cache poisoning
j.getAStep() = checkout and
j.getAStep() = artifact and
(
// the job writes to the cache
// (No need to follow the checkout step as the cache writing is normally done after the job completes)
@@ -49,9 +53,9 @@ where
or
// the job executes checked-out code
// (The cache specific token can be leaked even for non-privileged workflows)
checkout.getAFollowingStep() = s and
artifact.getAFollowingStep() = s and
s instanceof PoisonableStep and
// excluding privileged workflows since they can be exploited in easier circumstances
not j.isPrivileged()
)
select s, checkout, s, "Potential cache poisoning in the context of the default branch"
select s, artifact, s, "Potential cache poisoning in the context of the default branch"

2
ql/src/qlpack.yml
View File

@@ -1,7 +1,7 @@
---
library: false
name: github/actions-queries
version: 0.1.21
version: 0.1.22
groups: [actions, queries]
suites: codeql-suites
extractor: javascript

15
ql/test/query-tests/Security/CWE-094/.github/workflows/untrusted_checkout1.yml vendored Normal file
View File

@@ -0,0 +1,15 @@
on:
pull_request_target
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
ref: ${{ github.event.pull_request.head.sha }}
- id: artifact
run: |
echo "::set-output name=pr_number::$(<artifact.txt)"
- name: Use artifact
run: echo ${{ steps.artifact.outputs.pr_number }}

8
ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected
View File

@@ -106,6 +106,9 @@ edges
| .github/workflows/test.yml:34:20:34:48 | steps.step3.outputs.MSG3 | .github/workflows/test.yml:32:9:36:6 | Run Step: step4 [MSG4] | provenance | |
| .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | provenance | |
| .github/workflows/test.yml:38:20:38:48 | steps.step4.outputs.MSG4 | .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | provenance | |
| .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | provenance | |
| .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | provenance | |
| .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | provenance | |
nodes
| .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
| .github/actions/action3/action.yml:9:19:9:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
@@ -332,6 +335,10 @@ nodes
| .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | semmle.label | Run Step: step5 [MSG5] |
| .github/workflows/test.yml:38:20:38:48 | steps.step4.outputs.MSG4 | semmle.label | steps.step4.outputs.MSG4 |
| .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] | semmle.label | needs.job1.outputs['job_output'] |
| .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | semmle.label | Run Step: artifact [pr_number] |
| .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | semmle.label | echo "::set-output name=pr_number::$(<artifact.txt)"\n |
| .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | semmle.label | steps.artifact.outputs.pr_number |
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | semmle.label | github.event.workflow_run.display_title |
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | semmle.label | github.event.workflow_run.head_commit.message |
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | semmle.label | github.event.workflow_run.head_commit.author.email |
@@ -435,6 +442,7 @@ subpaths
| .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:27:18:27:75 | fromJson(needs.parse-issue.outputs.payload).version | ${{ fromJson(needs.parse-issue.outputs.payload).version }} |
| .github/workflows/test9.yml:31:42:31:99 | fromJson(needs.parse-issue.outputs.payload).version | .github/workflows/test9.yml:12:9:20:6 | Uses Step: issue_body_parser_request | .github/workflows/test9.yml:31:42:31:99 | fromJson(needs.parse-issue.outputs.payload).version | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:31:42:31:99 | fromJson(needs.parse-issue.outputs.payload).version | ${{ fromJson(needs.parse-issue.outputs.payload).version }} |
| .github/workflows/test9.yml:39:42:39:72 | github.event.issue.title | .github/workflows/test9.yml:39:42:39:72 | github.event.issue.title | .github/workflows/test9.yml:39:42:39:72 | github.event.issue.title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test9.yml:39:42:39:72 | github.event.issue.title | ${{ github.event.issue.title }} |
| .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | ${{ steps.artifact.outputs.pr_number }} |
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | ${{ github.event.workflow_run.display_title }} |
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | ${{ github.event.workflow_run.head_commit.message }} |
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | ${{ github.event.workflow_run.head_commit.author.email }} |

7
ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected
View File

@@ -106,6 +106,9 @@ edges
| .github/workflows/test.yml:34:20:34:48 | steps.step3.outputs.MSG3 | .github/workflows/test.yml:32:9:36:6 | Run Step: step4 [MSG4] | provenance | |
| .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | .github/workflows/test.yml:8:20:8:50 | steps.step5.outputs.MSG5 | provenance | |
| .github/workflows/test.yml:38:20:38:48 | steps.step4.outputs.MSG4 | .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | provenance | |
| .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | provenance | |
| .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | provenance | |
| .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | provenance | |
nodes
| .github/actions/action1/action.yml:7:19:7:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
| .github/actions/action3/action.yml:9:19:9:55 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
@@ -332,6 +335,10 @@ nodes
| .github/workflows/test.yml:36:9:41:2 | Run Step: step5 [MSG5] | semmle.label | Run Step: step5 [MSG5] |
| .github/workflows/test.yml:38:20:38:48 | steps.step4.outputs.MSG4 | semmle.label | steps.step4.outputs.MSG4 |
| .github/workflows/test.yml:49:20:49:56 | needs.job1.outputs['job_output'] | semmle.label | needs.job1.outputs['job_output'] |
| .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | semmle.label | Uses Step |
| .github/workflows/untrusted_checkout1.yml:11:9:14:6 | Run Step: artifact [pr_number] | semmle.label | Run Step: artifact [pr_number] |
| .github/workflows/untrusted_checkout1.yml:12:14:13:63 | echo "::set-output name=pr_number::$(<artifact.txt)"\n | semmle.label | echo "::set-output name=pr_number::$(<artifact.txt)"\n |
| .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | semmle.label | steps.artifact.outputs.pr_number |
| .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | semmle.label | github.event.workflow_run.display_title |
| .github/workflows/workflow_run.yml:10:19:10:70 | github.event.workflow_run.head_commit.message | semmle.label | github.event.workflow_run.head_commit.message |
| .github/workflows/workflow_run.yml:11:19:11:75 | github.event.workflow_run.head_commit.author.email | semmle.label | github.event.workflow_run.head_commit.author.email |