Skip to content

Commit 5bfe2a9

Browse files
redsun82redsun82
authored
Merge branch 'main' into redsun82/rust-cli-flags
2 parents 1b3a5cd + f9e4c0a commit 5bfe2a9

10 files changed

Lines changed: 273 additions & 4 deletions

File tree

‎.github/pull_request_template.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@
1111

1212
- [ ] Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to `.ql`, `.qll`, or `.qhelp` files. See [the documentation](https://github.com/github/codeql-team/blob/main/docs/best-practices/validating-autofix-for-query-changes.md) (internal access required).
1313
- [ ] Changes are validated [at scale](https://github.com/github/codeql-dca/) (internal access required).
14+
- [ ] Adding a new query? Consider also [adding the query to autofix](https://github.com/github/codeml-autofix/blob/main/docs/updating-query-support.md#adding-a-new-query-to-the-query-suite).

‎docs/prepare-db-upgrade.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,16 @@ extended.rel: reorder input.rel (int id, string name, int parent) id name parent
5959
// QLL library, and will run in the context of the *old* dbscheme.
6060
relationname.rel: run relationname.qlo
6161
62-
// Create relationname.rel by running the query predicate 'predicatename' in
63-
// relationname.qlo and writing the query results as a .rel file. This command
62+
// Create relation1.rel by running the query predicate 'predicate1' in upgrade.qlo
63+
// and writing the query results as a .rel file, and running 'predicate2' in
64+
// upgrade.qlo and writing the query results as a .rel file. This command
6465
// expects the upgrade relation to be a query predicate, which has the advantage
6566
// of allowing multiple upgrade relations to appear in the same .ql file as
66-
// multiple query predicates. The query file should be named relationname.ql and
67+
// multiple query predicates. The query file should be named upgrade.ql and
6768
// should be placed in the upgrade directory. It should avoid using the default
6869
// QLL library, and will run in the context of the *old* dbscheme.
69-
relationname.rel: run relationname.qlo predicatename
70+
relation1.rel: run upgrade.qlo predicate1
71+
relation2.rel: run upgrade.qlo predicate2
7072
```
7173

7274
### Testing your scripts

‎javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Sensitive information included in a GitHub Actions artifact can allow an attacker to access
8+
the sensitive information if the artifact is published.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
Only store information that is meant to be publicly available in a GitHub Actions artifact.
15+
</p>
16+
</recommendation>
17+
18+
<example>
19+
<p>
20+
The following example uses <code>actions/checkout</code> to checkout code which stores the GITHUB_TOKEN in the `.git/config` file
21+
and then stores the contents of the `.git` repository into the artifact:
22+
</p>
23+
<sample src="examples/actions-artifact-leak.yml"/>
24+
<p>
25+
The issue has been fixed below, where the <code>actions/upload-artifact</code> uses a version (v4+) which does not include hidden files or
26+
directories into the artifact.
27+
</p>
28+
<sample src="examples/actions-artifact-leak-fixed.yml"/>
29+
</example>
30+
</qhelp>

‎javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* @name Storage of sensitive information in GitHub Actions artifact
3+
* @description Including sensitive information in a GitHub Actions artifact can
4+
* expose it to an attacker.
5+
* @kind problem
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id js/actions/actions-artifact-leak
10+
* @tags security
11+
* external/cwe/cwe-312
12+
* external/cwe/cwe-315
13+
* external/cwe/cwe-359
14+
*/
15+
16+
import javascript
17+
import semmle.javascript.Actions
18+
19+
/**
20+
* A step that uses `actions/checkout` action.
21+
*/
22+
class ActionsCheckoutStep extends Actions::Step {
23+
ActionsCheckoutStep() { this.getUses().getGitHubRepository() = "actions/checkout" }
24+
}
25+
26+
/**
27+
* A `with:`/`persist-credentials` field sibling to `uses: actions/checkout`.
28+
*/
29+
class ActionsCheckoutWithPersistCredentials extends YamlNode, YamlScalar {
30+
ActionsCheckoutStep step;
31+
32+
ActionsCheckoutWithPersistCredentials() {
33+
step.lookup("with").(YamlMapping).lookup("persist-credentials") = this
34+
}
35+
36+
/** Gets the step this field belongs to. */
37+
ActionsCheckoutStep getStep() { result = step }
38+
}
39+
40+
/**
41+
* A `with:`/`path` field sibling to `uses: actions/checkout`.
42+
*/
43+
class ActionsCheckoutWithPath extends YamlNode, YamlString {
44+
ActionsCheckoutStep step;
45+
46+
ActionsCheckoutWithPath() { step.lookup("with").(YamlMapping).lookup("path") = this }
47+
48+
/** Gets the step this field belongs to. */
49+
ActionsCheckoutStep getStep() { result = step }
50+
}
51+
52+
/**
53+
* A step that uses `actions/upload-artifact` action.
54+
*/
55+
class ActionsUploadArtifactStep extends Actions::Step {
56+
ActionsUploadArtifactStep() { this.getUses().getGitHubRepository() = "actions/upload-artifact" }
57+
}
58+
59+
/**
60+
* A `with:`/`path` field sibling to `uses: actions/upload-artifact`.
61+
*/
62+
class ActionsUploadArtifactWithPath extends YamlNode, YamlString {
63+
ActionsUploadArtifactStep step;
64+
65+
ActionsUploadArtifactWithPath() { step.lookup("with").(YamlMapping).lookup("path") = this }
66+
67+
/** Gets the step this field belongs to. */
68+
ActionsUploadArtifactStep getStep() { result = step }
69+
}
70+
71+
from ActionsCheckoutStep checkout, ActionsUploadArtifactStep upload, Actions::Job job, int i, int j
72+
where
73+
checkout.getJob() = job and
74+
upload.getJob() = job and
75+
job.getStep(i) = checkout and
76+
job.getStep(j) = upload and
77+
j = i + 1 and
78+
upload.getUses().getVersion() =
79+
[
80+
"v4.3.6", "834a144ee995460fba8ed112a2fc961b36a5ec5a", //
81+
"v4.3.5", "89ef406dd8d7e03cfd12d9e0a4a378f454709029", //
82+
"v4.3.4", "0b2256b8c012f0828dc542b3febcab082c67f72b", //
83+
"v4.3.3", "65462800fd760344b1a7b4382951275a0abb4808", //
84+
"v4.3.2", "1746f4ab65b179e0ea60a494b83293b640dd5bba", //
85+
"v4.3.1", "5d5d22a31266ced268874388b861e4b58bb5c2f3", //
86+
"v4.3.0", "26f96dfa697d77e81fd5907df203aa23a56210a8", //
87+
"v4.2.0", "694cdabd8bdb0f10b2cea11669e1bf5453eed0a6", //
88+
"v4.1.0", "1eb3cb2b3e0f29609092a73eb033bb759a334595", //
89+
"v4.0.0", "c7d193f32edcb7bfad88892161225aeda64e9392", //
90+
] and
91+
(
92+
not exists(ActionsCheckoutWithPersistCredentials persist | persist.getStep() = checkout)
93+
or
94+
exists(ActionsCheckoutWithPersistCredentials persist |
95+
persist.getStep() = checkout and
96+
persist.getValue() = "true"
97+
)
98+
) and
99+
(
100+
not exists(ActionsCheckoutWithPath path | path.getStep() = checkout) and
101+
exists(ActionsUploadArtifactWithPath path |
102+
path.getStep() = upload and path.getValue() = [".", "*"]
103+
)
104+
or
105+
exists(ActionsCheckoutWithPath checkout_path, ActionsUploadArtifactWithPath upload_path |
106+
checkout_path.getValue() + ["", "/*"] = upload_path.getValue() and
107+
checkout_path.getStep() = checkout and
108+
upload_path.getStep() = upload
109+
)
110+
)
111+
select upload, "A secret may be exposed in an artifact."

‎javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak-fixed.yml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
name: secrets-in-artifacts
2+
on:
3+
pull_request:
4+
jobs:
5+
a-job: # NOT VULNERABLE
6+
runs-on: ubuntu-latest
7+
steps:
8+
- uses: actions/checkout@v4
9+
- name: "Upload artifact"
10+
uses: actions/upload-artifact@v4
11+
with:
12+
name: file
13+
path: .
14+

‎javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
name: secrets-in-artifacts
2+
on:
3+
pull_request:
4+
jobs:
5+
a-job: # VULNERABLE
6+
runs-on: ubuntu-latest
7+
steps:
8+
- uses: actions/checkout@v4
9+
- name: "Upload artifact"
10+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
11+
with:
12+
name: file
13+
path: .

‎javascript/ql/src/change-notes/2024-09-06-new-actions-artifact-leak-query.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
5+
- Added a new query (`js/actions/actions-artifact-leak`) to detect GitHub Actions artifacts that may leak the GITHUB_TOKEN token.

‎javascript/ql/test/query-tests/Security/CWE-312/.github/workflows/test.yml‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
name: secrets-in-artifacts
2+
on:
3+
pull_request:
4+
jobs:
5+
test1: # VULNERABLE
6+
runs-on: ubuntu-latest
7+
steps:
8+
- uses: actions/checkout@v4
9+
- name: "Upload artifact"
10+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
11+
with:
12+
name: file
13+
path: .
14+
test2: # NOT VULNERABLE
15+
runs-on: ubuntu-latest
16+
steps:
17+
- uses: actions/checkout@v4
18+
- name: "Upload artifact"
19+
uses: actions/upload-artifact@v4
20+
with:
21+
name: file
22+
path: .
23+
test3: # VULNERABLE
24+
runs-on: ubuntu-latest
25+
steps:
26+
- uses: actions/checkout@v4
27+
- name: "Upload artifact"
28+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
29+
with:
30+
name: file
31+
path: "*"
32+
test4: # VULNERABLE
33+
runs-on: ubuntu-latest
34+
steps:
35+
- uses: actions/checkout@v4
36+
with:
37+
path: foo
38+
- name: "Upload artifact"
39+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
40+
with:
41+
name: file
42+
path: foo
43+
test5: # VULNERABLE
44+
runs-on: ubuntu-latest
45+
steps:
46+
- uses: actions/checkout@v4
47+
with:
48+
path: foo
49+
- name: "Upload artifact"
50+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
51+
with:
52+
name: file
53+
path: foo/*
54+
test6: # NOT VULNERABLE
55+
runs-on: ubuntu-latest
56+
steps:
57+
- uses: actions/checkout@v4
58+
with:
59+
path: pr
60+
- name: "Upload artifact"
61+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
62+
with:
63+
name: file
64+
path: foo
65+
test7: # NOT VULNERABLE
66+
runs-on: ubuntu-latest
67+
steps:
68+
- uses: actions/checkout@v4
69+
with:
70+
persist-credentials: false
71+
- name: "Upload artifact"
72+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
73+
with:
74+
name: file
75+
path: .
76+
test8: # VULNERABLE
77+
runs-on: ubuntu-latest
78+
steps:
79+
- uses: actions/checkout@v4
80+
with:
81+
persist-credentials: true
82+
- name: "Upload artifact"
83+
uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2
84+
with:
85+
name: file
86+
path: .
87+

‎javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| .github/workflows/test.yml:9:9:14:2 | name: " ... tifact" | A secret may be exposed in an artifact. |
2+
| .github/workflows/test.yml:27:9:32:2 | name: " ... tifact" | A secret may be exposed in an artifact. |
3+
| .github/workflows/test.yml:38:9:43:2 | name: " ... tifact" | A secret may be exposed in an artifact. |
4+
| .github/workflows/test.yml:49:9:54:2 | name: " ... tifact" | A secret may be exposed in an artifact. |
5+
| .github/workflows/test.yml:82:9:86:18 | name: " ... tifact" | A secret may be exposed in an artifact. |

‎javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-312/ActionsArtifactLeak.ql

0 commit comments

Comments
 (0)