Skip to content

Commit c14d069

Browse files
author
Alvaro Muñoz
authored
Merge pull request #5 from github/cache_poisoning
Add Cache Poisoning Query
2 parents 2980139 + f6b1daa commit c14d069

File tree

11 files changed

+239
-0
lines changed

11 files changed

+239
-0
lines changed

‎ql/lib/codeql/actions/security/CachePoisoningQuery.qll

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import actions
2+
3+
abstract class CacheWritingStep extends Step { }
4+
5+
class CacheActionUsesStep extends CacheWritingStep, UsesStep {
6+
CacheActionUsesStep() { this.getCallee() = "actions/cache" }
7+
}
8+
9+
class CacheActionSaveUsesStep extends CacheWritingStep, UsesStep {
10+
CacheActionSaveUsesStep() { this.getCallee() = "actions/cache/save" }
11+
}
12+
13+
class SetupJavaUsesStep extends CacheWritingStep, UsesStep {
14+
SetupJavaUsesStep() {
15+
this.getCallee() = "actions/setup-java" and
16+
(
17+
exists(this.getArgument("cache")) or
18+
exists(this.getArgument("cache-dependency-path"))
19+
)
20+
}
21+
}
22+
23+
class SetupGoUsesStep extends CacheWritingStep, UsesStep {
24+
SetupGoUsesStep() {
25+
this.getCallee() = "actions/setup-go" and
26+
(
27+
not exists(this.getArgument("cache"))
28+
or
29+
this.getArgument("cache") = "true"
30+
)
31+
}
32+
}
33+
34+
class SetupNodeUsesStep extends CacheWritingStep, UsesStep {
35+
SetupNodeUsesStep() {
36+
this.getCallee() = "actions/setup-node" and
37+
(
38+
exists(this.getArgument("cache")) or
39+
exists(this.getArgument("cache-dependency-path"))
40+
)
41+
}
42+
}
43+
44+
class SetupPythonUsesStep extends CacheWritingStep, UsesStep {
45+
SetupPythonUsesStep() {
46+
this.getCallee() = "actions/setup-python" and
47+
(
48+
exists(this.getArgument("cache")) or
49+
exists(this.getArgument("cache-dependency-path"))
50+
)
51+
}
52+
}
53+
54+
class SetupDotnetUsesStep extends CacheWritingStep, UsesStep {
55+
SetupDotnetUsesStep() {
56+
this.getCallee() = "actions/setup-dotnet" and
57+
(
58+
this.getArgument("cache") = "true" or
59+
exists(this.getArgument("cache-dependency-path"))
60+
)
61+
}
62+
}

‎ql/src/Security/CWE-349/CachePoisoning.ql

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @name Cache Poisoning
3+
* @description The cache can be poisoned by untrusted code, leading to a cache poisoning attack.
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision high
7+
* @security-severity 9.3
8+
* @id actions/cache-poisoning
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-349
12+
*/
13+
14+
import actions
15+
import codeql.actions.security.UntrustedCheckoutQuery
16+
import codeql.actions.security.CachePoisoningQuery
17+
import codeql.actions.security.PoisonableSteps
18+
19+
from LocalJob j, PRHeadCheckoutStep checkout
20+
where
21+
// The workflow runs in the context of the default branch
22+
// TODO: (require to collect trigger types)
23+
// - add push to default branch?
24+
// - exclude pull_request_target when branches_ignore includes default branch or when branches does not include the default branch
25+
j.getEnclosingWorkflow()
26+
.hasTriggerEvent([
27+
"check_run", "check_suite", "delete", "discussion", "discussion_comment", "fork",
28+
"gollum", "issue_comment", "issues", "label", "milestone", "project", "project_card",
29+
"project_column", "public", "pull_request_comment", "pull_request_target",
30+
"repository_dispatch", "schedule", "watch", "workflow_run"
31+
]) and
32+
// The job checkouts untrusted code from a pull request
33+
j.getAStep() = checkout and
34+
(
35+
// The job writes to the cache
36+
// (No need to follow the checkout step as the cache writing is normally done after the job completes)
37+
j.getAStep() instanceof CacheWritingStep
38+
or
39+
// The job executes checked-out code
40+
// (The cache specific token can be leaked even for non-privileged workflows)
41+
checkout.getAFollowingStep() instanceof PoisonableStep
42+
)
43+
select j.getAStep().(CacheWritingStep), "Potential cache poisoning on privileged workflow."

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test1.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
on:
2+
issue_comment:
3+
types: [created]
4+
5+
jobs:
6+
pr-comment:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: xt0rted/pull-request-comment-branch@v2
10+
id: comment-branch
11+
12+
- uses: actions/checkout@v3
13+
if: success()
14+
with:
15+
ref: ${{ steps.comment-branch.outputs.head_sha }}
16+
17+
- uses: actions/cache@v2
18+
with:
19+
path: ./poison
20+
key: poison_key
21+
- run: |
22+
cat poison

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test2.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/cache@v2
13+
with:
14+
path: ./poison
15+
key: poison_key
16+
- run: |
17+
cat poison

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test3.yml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/setup-java@v2
13+
with:
14+
distribution: 'zulu'
15+
java-version: '21'
16+
cache: 'gradle'
17+
cache-dependency-path: |
18+
sub-project/*.gradle*
19+
sub-project/**/gradle-wrapper.properties
20+
- run: |
21+
java HelloWorldApp.java

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test4.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/setup-java@v2
13+
with:
14+
distribution: 'zulu'
15+
java-version: '21'
16+
- run: |
17+
java HelloWorldApp.java

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test5.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/setup-go@v2
13+
with:
14+
go-version-file: 'go.mod'
15+
cache: false
16+
- run: do some go stuff
17+

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test6.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/setup-go@v2
13+
with:
14+
go-version-file: 'go.mod'
15+
cache: true
16+
- run: do some go stuff
17+

‎ql/test/query-tests/Security/CWE-349/.github/workflows/test7.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
name: Cache Poisoning
2+
3+
on: pull_request_target
4+
5+
jobs:
6+
poison:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v3
10+
with:
11+
ref: ${{ github.event.pull_request.head.sha }}
12+
- uses: actions/setup-go@v2
13+
with:
14+
go-version-file: 'go.mod'
15+
- run: do some go stuff
16+

‎ql/test/query-tests/Security/CWE-349/CachePoisoning.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| .github/workflows/test1.yml:17:9:21:6 | Uses Step | Potential cache poisoning on privileged workflow. |
2+
| .github/workflows/test2.yml:12:9:16:6 | Uses Step | Potential cache poisoning on privileged workflow. |
3+
| .github/workflows/test3.yml:12:9:20:6 | Uses Step | Potential cache poisoning on privileged workflow. |
4+
| .github/workflows/test6.yml:12:9:16:6 | Uses Step | Potential cache poisoning on privileged workflow. |
5+
| .github/workflows/test7.yml:12:9:15:6 | Uses Step | Potential cache poisoning on privileged workflow. |

0 commit comments

Comments
 (0)