Skip to content

Commit 3ba6b85

Browse files
committed
Harden workspace execution path normalization
1 parent b324dad commit 3ba6b85

8 files changed

Lines changed: 107 additions & 18 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* Added `LocalScriptExecutionRunStep.getRawPath()` to expose a captured script path before workspace qualification.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improved normalization of quoted and dot-relative paths used by Actions security queries.

actions/ql/lib/codeql/actions/security/PoisonableSteps.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,13 @@ class LocalScriptExecutionRunStep extends PoisonableStep, Run {
4747
}
4848

4949
/** Gets the captured path before workspace qualification. */
50-
string getRawPath() { result = path.splitAt(" ") }
50+
string getRawPath() {
51+
if path.regexpMatch("^['\"]?\\$\\{\\{.*")
52+
then
53+
// Expression whitespace belongs to the path operand; other models retain first-token behavior.
54+
result = path
55+
else result = path.splitAt(" ")
56+
}
5157

5258
string getPath() { result = normalizePath(this.getRawPath()) }
5359
}

actions/ql/lib/ext/config/poisonable_steps.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ extensions:
6767
# TODO: It could also be in the form of `dir/cmd`
6868
- ["(\\.\\/[^\\s]+)\\b", 1] # eg: ./venv/bin/activate
6969
- ["(\\.\\s+[^\\s]+)\\b", 1] # eg: . venv/bin/activate
70-
- ["(source|sh|bash|zsh|fish)\\s+([^\\s]+)\\b", 2]
70+
# An Actions expression may contain spaces while remaining one shell operand.
71+
- ["(source|sh|bash|zsh|fish)\\s+((?:['\"]?\\$\\{\\{(?:[^}]|}(?!}))*+\\}\\}['\"]?[^\\s]*|[^\\s]+)\\b[\"']?)", 2]
7172
- ["(node)\\s+([^\\s]+)(\\.js|\\.ts)\\b", 2]
7273
- ["(python[\\d\\.]*)\\s+([^\\s]+)\\.py\\b", 2]
7374
- ["(python[\\d\\.]*)\\s+-m\\s+([A-Za-z_][\\w\\.]*)\\b", 2] # eg: pythonX -m anything(dir or file)

actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,50 @@ private class ActionsCheckoutPathInput extends NormalizableFilepath {
3131
}
3232
}
3333

34+
bindingset[path]
35+
private predicate isWorkspaceEnvironmentExecutionPath(string path) {
36+
// Shell variables expand when unquoted or double quoted, but not when single quoted.
37+
path.regexpMatch([
38+
"^\\$GITHUB_WORKSPACE/.*", "^\\$\\{GITHUB_WORKSPACE\\}/.*", "^\"\\$GITHUB_WORKSPACE/.*\"$",
39+
"^\"\\$\\{GITHUB_WORKSPACE\\}/.*\"$", "^\"\\$GITHUB_WORKSPACE\"/.*",
40+
"^\"\\$\\{GITHUB_WORKSPACE\\}\"/.*"
41+
])
42+
}
43+
44+
bindingset[path]
45+
private predicate isWorkspaceContextExecutionPath(string path) {
46+
// Actions evaluates contexts before the resulting command is passed to the shell.
47+
path.regexpMatch([
48+
"^\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}/.*",
49+
"^\"\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}/.*\"$",
50+
"^'\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}/.*'$",
51+
"^\"\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}\"/.*",
52+
"^'\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}'/.*"
53+
])
54+
}
55+
3456
bindingset[path]
3557
private string normalizeWorkspaceExecutionPath(string path) {
36-
// Only GitHub's workspace variable has known path provenance; leave other variables unresolved.
37-
if path.indexOf("$GITHUB_WORKSPACE/") = 0
38-
then result = path.regexpReplaceAll("^\\$GITHUB_WORKSPACE/", "GITHUB_WORKSPACE/")
39-
else
40-
if path.indexOf("${GITHUB_WORKSPACE}/") = 0
41-
then result = path.regexpReplaceAll("^\\$\\{GITHUB_WORKSPACE\\}/", "GITHUB_WORKSPACE/")
42-
else result = path
58+
isWorkspaceEnvironmentExecutionPath(path) and
59+
result =
60+
path.regexpReplaceAll("^\"?\\$GITHUB_WORKSPACE\"?/", "GITHUB_WORKSPACE/")
61+
.regexpReplaceAll("^\"?\\$\\{GITHUB_WORKSPACE\\}\"?/", "GITHUB_WORKSPACE/")
62+
.regexpReplaceAll("\"$", "")
63+
or
64+
isWorkspaceContextExecutionPath(path) and
65+
result =
66+
path.regexpReplaceAll("^[\"']?\\$\\{\\{\\s*github\\.workspace\\s*\\}\\}[\"']?/",
67+
"GITHUB_WORKSPACE/").regexpReplaceAll("[\"']$", "")
4368
}
4469

4570
private class ExecutionPathInput extends NormalizableFilepath {
4671
ExecutionPathInput() {
4772
exists(LocalScriptExecutionRunStep step |
48-
this = normalizeWorkspaceExecutionPath(trimQuotes(step.getRawPath()))
73+
this = step.getRawPath()
74+
or
75+
this = trimQuotes(step.getRawPath())
76+
or
77+
this = normalizeWorkspaceExecutionPath(step.getRawPath())
4978
)
5079
or
5180
exists(LocalActionUsesStep step | this = step.getCallee())
@@ -70,17 +99,19 @@ private string getNormalizedActionsCheckoutPath(PRHeadCheckoutStep checkout) {
7099

71100
bindingset[path]
72101
private string getNormalizedExecutionPath(string path) {
73-
exists(ExecutionPathInput executionPath, string rawPath, string normalized |
74-
rawPath = trimQuotes(path) and
75-
executionPath = normalizeWorkspaceExecutionPath(rawPath) and
102+
exists(ExecutionPathInput executionPath, string normalized |
103+
(
104+
executionPath = normalizeWorkspaceExecutionPath(path)
105+
or
106+
not exists(normalizeWorkspaceExecutionPath(path)) and
107+
executionPath = trimQuotes(path)
108+
) and
76109
not executionPath.regexpMatch(".*\\$\\{\\{.*") and
77110
normalized = executionPath.getNormalizedPath() and
78111
not normalized.matches("/%") and
79112
normalized != ".." and
80113
not normalized.matches("../%") and
81-
if
82-
rawPath.indexOf("$GITHUB_WORKSPACE/") = 0 or
83-
rawPath.indexOf("${GITHUB_WORKSPACE}/") = 0
114+
if exists(normalizeWorkspaceExecutionPath(path))
84115
then
85116
(normalized = "GITHUB_WORKSPACE" or normalized.matches("GITHUB_WORKSPACE/%")) and
86117
result = normalized
@@ -124,6 +155,11 @@ where
124155
(
125156
// Check if the poisonable step is a local script execution step
126157
// and the path of the command or script matches the path of the downloaded artifact
158+
// A shell assignment breaks the default-workspace provenance of the environment variable.
159+
not (
160+
isWorkspaceEnvironmentExecutionPath(poisonable.(LocalScriptExecutionRunStep).getRawPath()) and
161+
poisonable.(LocalScriptExecutionRunStep).getScript().getAnAssignment("GITHUB_WORKSPACE", _)
162+
) and
127163
checkoutContainsPath(checkout, poisonable.(LocalScriptExecutionRunStep).getRawPath(),
128164
poisonable.(LocalScriptExecutionRunStep).getPath())
129165
or
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* The `actions/untrusted-checkout/critical` query now recognizes scripts and local actions executed from explicit relative `actions/checkout` paths without matching sibling directories.
4+
* The `actions/untrusted-checkout/critical` query now recognizes scripts and local actions executed from explicit relative `actions/checkout` paths, including workspace-root spellings, without matching sibling or escaping paths.

actions/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout_paths.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,19 @@ jobs:
204204
- run: bash ${GITHUB_WORKSPACE}/candidate-sibling/build.sh
205205
- run: bash ${GITHUB_WORKSPACE}/candidate/../trusted.sh
206206
- run: bash ${GITHUB_WORKSPACE}/../candidate/build.sh
207+
- run: bash "$GITHUB_WORKSPACE/candidate/build.sh"
208+
- run: bash "$GITHUB_WORKSPACE"/candidate/build.sh
209+
- run: bash "${GITHUB_WORKSPACE}"/candidate/build.sh
210+
- run: bash '$GITHUB_WORKSPACE/candidate/build.sh'
211+
- run: |
212+
GITHUB_WORKSPACE=/tmp
213+
bash $GITHUB_WORKSPACE/candidate/build.sh
214+
- run: bash ${{ github.workspace }}/candidate/build.sh
215+
- run: bash "${{ github.workspace }}/candidate/build.sh"
216+
- run: bash '${{ github.workspace }}/candidate/build.sh'
217+
- run: bash "${{ github.workspace }}"/candidate/build.sh
218+
- run: bash ${{github.workspace}}/candidate/build.sh
219+
- run: bash ${{ github.workspace }}/candidate-sibling/build.sh
220+
- run: bash ${{ github.workspace }}/candidate/../trusted.sh
221+
- run: bash ${{ github.workspace }}/../candidate/build.sh
222+
- run: bash ${{ github.action_path }}/candidate/build.sh

actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,21 @@ edges
364364
| .github/workflows/untrusted_checkout_paths.yml:202:9:203:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:203:9:204:6 | Run Step |
365365
| .github/workflows/untrusted_checkout_paths.yml:203:9:204:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:204:9:205:6 | Run Step |
366366
| .github/workflows/untrusted_checkout_paths.yml:204:9:205:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:205:9:206:6 | Run Step |
367-
| .github/workflows/untrusted_checkout_paths.yml:205:9:206:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:206:9:206:60 | Run Step |
367+
| .github/workflows/untrusted_checkout_paths.yml:205:9:206:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:206:9:207:6 | Run Step |
368+
| .github/workflows/untrusted_checkout_paths.yml:206:9:207:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:207:9:208:6 | Run Step |
369+
| .github/workflows/untrusted_checkout_paths.yml:207:9:208:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:208:9:209:6 | Run Step |
370+
| .github/workflows/untrusted_checkout_paths.yml:208:9:209:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:209:9:210:6 | Run Step |
371+
| .github/workflows/untrusted_checkout_paths.yml:209:9:210:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:210:9:211:6 | Run Step |
372+
| .github/workflows/untrusted_checkout_paths.yml:210:9:211:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:211:9:214:6 | Run Step |
373+
| .github/workflows/untrusted_checkout_paths.yml:211:9:214:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:214:9:215:6 | Run Step |
374+
| .github/workflows/untrusted_checkout_paths.yml:214:9:215:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:215:9:216:6 | Run Step |
375+
| .github/workflows/untrusted_checkout_paths.yml:215:9:216:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:216:9:217:6 | Run Step |
376+
| .github/workflows/untrusted_checkout_paths.yml:216:9:217:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:217:9:218:6 | Run Step |
377+
| .github/workflows/untrusted_checkout_paths.yml:217:9:218:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:218:9:219:6 | Run Step |
378+
| .github/workflows/untrusted_checkout_paths.yml:218:9:219:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:219:9:220:6 | Run Step |
379+
| .github/workflows/untrusted_checkout_paths.yml:219:9:220:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:220:9:221:6 | Run Step |
380+
| .github/workflows/untrusted_checkout_paths.yml:220:9:221:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:221:9:222:6 | Run Step |
381+
| .github/workflows/untrusted_checkout_paths.yml:221:9:222:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:222:9:222:63 | Run Step |
368382
| .github/workflows/untrusted_checkout_permission_check_reusable2.yml:8:9:16:6 | Uses Step: checkAccess | .github/workflows/untrusted_checkout_permission_check_reusable2.yml:16:9:22:2 | Run Step |
369383
| .github/workflows/untrusted_checkout_permission_check_reusable.yml:8:9:16:6 | Uses Step: checkAccess | .github/workflows/untrusted_checkout_permission_check_reusable.yml:16:9:22:2 | Run Step |
370384
| .github/workflows/untrusted_checkout_permission_check_reusable_level2.yml:8:9:16:6 | Uses Step: checkAccess | .github/workflows/untrusted_checkout_permission_check_reusable_level2.yml:16:9:22:2 | Run Step |
@@ -435,5 +449,13 @@ edges
435449
| .github/workflows/untrusted_checkout_paths.yml:163:9:168:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:163:9:168:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:168:9:170:2 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
436450
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:198:9:199:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
437451
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:203:9:204:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
452+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:207:9:208:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
453+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:208:9:209:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
454+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:209:9:210:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
455+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:214:9:215:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
456+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:215:9:216:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
457+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:216:9:217:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
458+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:217:9:218:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
459+
| .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:193:9:198:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:218:9:219:6 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_paths.yml:4:3:4:21 | pull_request_target | pull_request_target |
438460
| .github/workflows/untrusted_checkout_permissions_check.yml:36:9:41:6 | Uses Step | .github/workflows/untrusted_checkout_permissions_check.yml:36:9:41:6 | Uses Step | .github/workflows/untrusted_checkout_permissions_check.yml:41:9:41:22 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/untrusted_checkout_permissions_check.yml:2:3:2:21 | pull_request_target | pull_request_target |
439461
| .github/workflows/workflow_run_untrusted_checkout_path.yml:12:9:16:6 | Uses Step | .github/workflows/workflow_run_untrusted_checkout_path.yml:12:9:16:6 | Uses Step | .github/workflows/workflow_run_untrusted_checkout_path.yml:16:9:16:40 | Run Step | Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@). | .github/workflows/workflow_run_untrusted_checkout_path.yml:4:3:4:14 | workflow_run | workflow_run |

0 commit comments

Comments
 (0)