Skip to content

Commit a48c857

Browse files
1sgtpepper1sgtpepper
committed
Preserve checkout path semantics
1 parent b0fff43 commit a48c857

4 files changed

Lines changed: 71 additions & 15 deletions

File tree

‎actions/ql/lib/codeql/actions/Helper.qll‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,22 @@ bindingset[path]
6464
string normalizePath(string path) {
6565
exists(string trimmed_path | trimmed_path = trimQuotes(path) |
6666
// ./foo -> GITHUB_WORKSPACE/foo
67-
if path.indexOf("./") = 0
68-
then result = path.replaceAll("./", "GITHUB_WORKSPACE/")
67+
if trimmed_path.indexOf("./") = 0
68+
then
69+
// Only replace the leading component; `./` also occurs inside `../`.
70+
result = trimmed_path.regexpReplaceAll("^\\./", "GITHUB_WORKSPACE/")
6971
else
7072
// GITHUB_WORKSPACE/foo -> GITHUB_WORKSPACE/foo
71-
if path.indexOf("GITHUB_WORKSPACE/") = 0
72-
then result = path
73+
if trimmed_path.indexOf("GITHUB_WORKSPACE/") = 0
74+
then result = trimmed_path
7375
else
7476
// foo -> GITHUB_WORKSPACE/foo
75-
if path.regexpMatch("^[^$/~].*")
76-
then result = "GITHUB_WORKSPACE/" + path.regexpReplaceAll("/$", "")
77+
if trimmed_path.regexpMatch("^[^$/~].*")
78+
then result = "GITHUB_WORKSPACE/" + trimmed_path.regexpReplaceAll("/$", "")
7779
else
7880
// ~/foo -> ~/foo
7981
// /foo -> /foo
80-
result = path
82+
result = trimmed_path
8183
)
8284
}
8385

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ private class ActionsCheckoutPathInput extends NormalizableFilepath {
3333

3434
private class ExecutionPathInput extends NormalizableFilepath {
3535
ExecutionPathInput() {
36-
exists(LocalScriptExecutionRunStep step | this = trimQuotes(step.getPath()))
36+
exists(LocalScriptExecutionRunStep step | this = step.getPath())
3737
or
38-
exists(LocalActionUsesStep step | this = trimQuotes(step.getPath()))
38+
exists(LocalActionUsesStep step | this = step.getPath())
3939
or
4040
this = "GITHUB_WORKSPACE/"
4141
}
@@ -60,7 +60,7 @@ private string getNormalizedActionsCheckoutPath(PRHeadCheckoutStep checkout) {
6060
bindingset[path]
6161
private string getNormalizedExecutionPath(string path) {
6262
exists(ExecutionPathInput executionPath, string normalized |
63-
executionPath = trimQuotes(path) and
63+
executionPath = path and
6464
not executionPath.regexpMatch(".*\\$\\{\\{.*") and
6565
normalized = executionPath.getNormalizedPath() and
6666
(normalized = "GITHUB_WORKSPACE" or normalized.matches("GITHUB_WORKSPACE/%")) and
@@ -82,6 +82,13 @@ private predicate checkoutContainsPath(PRHeadCheckoutStep checkout, string path)
8282
isSubpath(path, checkout.getPath())
8383
}
8484

85+
private predicate checkoutUsesWorkspaceRoot(PRHeadCheckoutStep checkout) {
86+
getNormalizedActionsCheckoutPath(checkout) = "GITHUB_WORKSPACE"
87+
or
88+
not exists(getNormalizedActionsCheckoutPath(checkout)) and
89+
checkout.getPath() = "GITHUB_WORKSPACE/"
90+
}
91+
8592
from PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event
8693
where
8794
// the checkout is followed by a known poisonable step
@@ -102,7 +109,7 @@ where
102109
poisonable instanceof UsesStep and
103110
(
104111
not poisonable instanceof LocalActionUsesStep and
105-
checkoutContainsPath(checkout, "GITHUB_WORKSPACE/")
112+
checkoutUsesWorkspaceRoot(checkout)
106113
or
107114
checkoutContainsPath(checkout, poisonable.(LocalActionUsesStep).getPath())
108115
)

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,36 @@ jobs:
6464
path: checkout
6565
- run: bash checkout/../trusted.sh
6666

67+
leading-and-inner-dot-path:
68+
runs-on: ubuntu-latest
69+
steps:
70+
- uses: actions/checkout@v4.2.2
71+
with:
72+
repository: ${{ github.event.pull_request.head.repo.full_name }}
73+
ref: ${{ github.event.pull_request.head.sha }}
74+
path: a/b
75+
- run: bash ./a/./b/build.sh
76+
77+
quoted-candidate-path:
78+
runs-on: ubuntu-latest
79+
steps:
80+
- uses: actions/checkout@v4.2.2
81+
with:
82+
repository: ${{ github.event.pull_request.head.repo.full_name }}
83+
ref: ${{ github.event.pull_request.head.sha }}
84+
path: checkout-quoted
85+
- run: bash "checkout-quoted/build.sh"
86+
87+
leading-dot-escaped-candidate:
88+
runs-on: ubuntu-latest
89+
steps:
90+
- uses: actions/checkout@v4.2.2
91+
with:
92+
repository: ${{ github.event.pull_request.head.repo.full_name }}
93+
ref: ${{ github.event.pull_request.head.sha }}
94+
path: checkout
95+
- run: bash ./checkout/../trusted.sh
96+
6797
workspace-path:
6898
runs-on: ubuntu-latest
6999
steps:
@@ -86,6 +116,17 @@ jobs:
86116
with:
87117
cache: yarn
88118

119+
non-actions-literal-workspace-directory:
120+
runs-on: ubuntu-latest
121+
steps:
122+
- run: git fetch origin $HEAD_BRANCH
123+
working-directory: GITHUB_WORKSPACE
124+
env:
125+
HEAD_BRANCH: ${{ github.head_ref }}
126+
- uses: actions/setup-node@v4
127+
with:
128+
cache: yarn
129+
89130
sibling-directory:
90131
runs-on: ubuntu-latest
91132
steps:

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,13 @@ edges
345345
| .github/workflows/untrusted_checkout_paths.yml:50:9:55:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:55:9:57:2 | Run Step |
346346
| .github/workflows/untrusted_checkout_paths.yml:60:9:65:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:65:9:67:2 | Run Step |
347347
| .github/workflows/untrusted_checkout_paths.yml:70:9:75:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:75:9:77:2 | Run Step |
348-
| .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:85:9:89:2 | Uses Step |
349-
| .github/workflows/untrusted_checkout_paths.yml:92:9:97:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:97:9:99:2 | Run Step |
350-
| .github/workflows/untrusted_checkout_paths.yml:102:9:107:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:107:9:107:34 | Run Step |
348+
| .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:85:9:87:2 | Run Step |
349+
| .github/workflows/untrusted_checkout_paths.yml:90:9:95:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:95:9:97:2 | Run Step |
350+
| .github/workflows/untrusted_checkout_paths.yml:100:9:105:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:105:9:107:2 | Run Step |
351+
| .github/workflows/untrusted_checkout_paths.yml:110:9:115:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:115:9:119:2 | Uses Step |
352+
| .github/workflows/untrusted_checkout_paths.yml:122:9:126:6 | Run Step | .github/workflows/untrusted_checkout_paths.yml:126:9:130:2 | Uses Step |
353+
| .github/workflows/untrusted_checkout_paths.yml:133:9:138:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:138:9:140:2 | Run Step |
354+
| .github/workflows/untrusted_checkout_paths.yml:143:9:148:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:148:9:148:34 | Run Step |
351355
| .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 |
352356
| .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 |
353357
| .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 |
@@ -411,6 +415,8 @@ edges
411415
| .github/workflows/untrusted_checkout_paths.yml:40:9:45:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:40:9:45:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:45:9:47: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 |
412416
| .github/workflows/untrusted_checkout_paths.yml:50:9:55:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:50:9:55:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:55:9:57: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 |
413417
| .github/workflows/untrusted_checkout_paths.yml:70:9:75:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:70:9:75:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:75:9:77: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 |
414-
| .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:85:9:89:2 | Uses 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 |
418+
| .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:80:9:85:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:85:9:87: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 |
419+
| .github/workflows/untrusted_checkout_paths.yml:100:9:105:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:100:9:105:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:105:9:107: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 |
420+
| .github/workflows/untrusted_checkout_paths.yml:110:9:115:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:110:9:115:6 | Uses Step | .github/workflows/untrusted_checkout_paths.yml:115:9:119:2 | Uses 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 |
415421
| .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 |
416422
| .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)