Skip to content

Commit a5075e5

Browse files
author
Alvaro Muñoz
committed
Change queries to use the new bash parser
1 parent 2727bf5 commit a5075e5

8 files changed

Lines changed: 61 additions & 97 deletions

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink {
2828
)
2929
or
3030
exists(
31-
Run run, string line, string argument, string regexp, int argument_group, int command_group
31+
Run run, string cmd, string argument, string regexp, int argument_group, int command_group
3232
|
33-
run.getScript().splitAt("\n") = line and
33+
run.getACommand() = cmd and
3434
run.getScriptScalar() = this.asExpr() and
3535
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
36-
argument = line.regexpCapture(regexp, argument_group) and
37-
command = line.regexpCapture(regexp, command_group) and
36+
argument = cmd.regexpCapture(regexp, argument_group) and
37+
command = cmd.regexpCapture(regexp, command_group) and
3838
argument.regexpMatch(".*\\$(\\{)?(GITHUB_HEAD_REF).*")
3939
)
4040
}
@@ -60,12 +60,12 @@ private module ArgumentInjectionConfig implements DataFlow::ConfigSig {
6060
source instanceof RemoteFlowSource
6161
or
6262
exists(
63-
Run run, string argument, string line, string regexp, int command_group, int argument_group
63+
Run run, string argument, string cmd, string regexp, int command_group, int argument_group
6464
|
6565
run.getScriptScalar() = source.asExpr() and
66-
run.getScript().splitAt("\n") = line and
66+
run.getACommand() = cmd and
6767
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
68-
argument = line.regexpCapture(regexp, argument_group) and
68+
argument = cmd.regexpCapture(regexp, argument_group) and
6969
argument.regexpMatch(".*\\$(\\{)?(GITHUB_HEAD_REF).*")
7070
)
7171
}

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

Lines changed: 20 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -155,71 +155,54 @@ class ActionsGitHubScriptDownloadStep extends UntrustedArtifactDownloadStep, Use
155155
}
156156

157157
override string getPath() {
158-
if
159-
this.getAFollowingStep()
160-
.(Run)
161-
.getScript()
162-
.splitAt("\n")
163-
.regexpMatch(unzipRegexp() + unzipDirArgRegexp())
158+
if this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp() + unzipDirArgRegexp())
164159
then
165160
result =
166161
normalizePath(trimQuotes(this.getAFollowingStep()
167162
.(Run)
168-
.getScript()
169-
.splitAt("\n")
163+
.getACommand()
170164
.regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 2)))
171165
else
172-
if this.getAFollowingStep().(Run).getScript().splitAt("\n").regexpMatch(unzipRegexp())
166+
if this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp())
173167
then result = "GITHUB_WORKSPACE/"
174168
else none()
175169
}
176170
}
177171

178172
class GHRunArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run {
179-
string script;
180-
181173
GHRunArtifactDownloadStep() {
182174
// eg: - run: gh run download ${{ github.event.workflow_run.id }} --repo "${GITHUB_REPOSITORY}" --name "artifact_name"
183-
this.getScript() = script and
184-
script.splitAt("\n").regexpMatch(".*gh\\s+run\\s+download.*") and
185-
script.splitAt("\n").matches("%github.event.workflow_run.id%") and
175+
this.getACommand().regexpMatch(".*gh\\s+run\\s+download.*") and
176+
this.getACommand().matches("%github.event.workflow_run.id%") and
186177
(
187-
script.splitAt("\n").regexpMatch(unzipRegexp()) or
188-
this.getAFollowingStep().(Run).getScript().splitAt("\n").regexpMatch(unzipRegexp())
178+
this.getACommand().regexpMatch(unzipRegexp()) or
179+
this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp())
189180
)
190181
}
191182

192183
override string getPath() {
193184
if
194-
this.getAFollowingStep()
195-
.(Run)
196-
.getScript()
197-
.splitAt("\n")
198-
.regexpMatch(unzipRegexp() + unzipDirArgRegexp()) or
199-
script.splitAt("\n").regexpMatch(unzipRegexp() + unzipDirArgRegexp())
185+
this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp() + unzipDirArgRegexp()) or
186+
this.getACommand().regexpMatch(unzipRegexp() + unzipDirArgRegexp())
200187
then
201188
result =
202-
normalizePath(trimQuotes(script
203-
.splitAt("\n")
189+
normalizePath(trimQuotes(this.getACommand()
204190
.regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 2))) or
205191
result =
206192
normalizePath(trimQuotes(this.getAFollowingStep()
207193
.(Run)
208-
.getScript()
209-
.splitAt("\n")
194+
.getACommand()
210195
.regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 2)))
211196
else
212197
if
213-
this.getAFollowingStep().(Run).getScript().splitAt("\n").regexpMatch(unzipRegexp()) or
214-
script.splitAt("\n").regexpMatch(unzipRegexp())
198+
this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp()) or
199+
this.getACommand().regexpMatch(unzipRegexp())
215200
then result = "GITHUB_WORKSPACE/"
216201
else none()
217202
}
218203
}
219204

220205
class DirectArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run {
221-
string script;
222-
223206
DirectArtifactDownloadStep() {
224207
// eg:
225208
// run: |
@@ -230,32 +213,25 @@ class DirectArtifactDownloadStep extends UntrustedArtifactDownloadStep, Run {
230213
// gh api $url > "$name.zip"
231214
// unzip -d "$name" "$name.zip"
232215
// done
233-
this.getScript() = script and
234-
script.splitAt("\n").matches("%github.event.workflow_run.artifacts_url%") and
216+
this.getACommand().matches("%github.event.workflow_run.artifacts_url%") and
235217
(
236-
script.splitAt("\n").regexpMatch(unzipRegexp()) or
237-
this.getAFollowingStep().(Run).getScript().splitAt("\n").regexpMatch(unzipRegexp())
218+
this.getACommand().regexpMatch(unzipRegexp()) or
219+
this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp())
238220
)
239221
}
240222

241223
override string getPath() {
242224
if
243-
script.splitAt("\n").regexpMatch(unzipRegexp() + unzipDirArgRegexp()) or
244-
this.getAFollowingStep()
245-
.(Run)
246-
.getScript()
247-
.splitAt("\n")
248-
.regexpMatch(unzipRegexp() + unzipDirArgRegexp())
225+
this.getACommand().regexpMatch(unzipRegexp() + unzipDirArgRegexp()) or
226+
this.getAFollowingStep().(Run).getACommand().regexpMatch(unzipRegexp() + unzipDirArgRegexp())
249227
then
250228
result =
251-
normalizePath(trimQuotes(script
252-
.splitAt("\n")
229+
normalizePath(trimQuotes(this.getACommand()
253230
.regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 2))) or
254231
result =
255232
normalizePath(trimQuotes(this.getAFollowingStep()
256233
.(Run)
257-
.getScript()
258-
.splitAt("\n")
234+
.getACommand()
259235
.regexpCapture(unzipRegexp() + unzipDirArgRegexp(), 2)))
260236
else result = "GITHUB_WORKSPACE/"
261237
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,13 @@ class PermissionActionCheck extends PermissionCheck instanceof UsesStep {
255255

256256
class BashCommentVsHeadDateCheck extends CommentVsHeadDateCheck, Run {
257257
BashCommentVsHeadDateCheck() {
258-
exists(string line |
259-
line = this.getScript().splitAt("\n") and
260-
line.toLowerCase()
261-
.regexpMatch(".*date\\s+-d.*(commit_at|pushed_at|comment_at|commented_at).*date\\s+-d.*(commit_at|pushed_at|comment_at|commented_at).*")
258+
// eg: if [[ $(date -d "$pushed_at" +%s) -gt $(date -d "$COMMENT_AT" +%s) ]]; then
259+
exists(string cmd1, string cmd2 |
260+
cmd1 = this.getACommand() and
261+
cmd2 = this.getACommand() and
262+
not cmd1 = cmd2 and
263+
cmd1.toLowerCase().regexpMatch("date\\s+-d.*(commit|pushed|comment|commented)_at.*") and
264+
cmd2.toLowerCase().regexpMatch("date\\s+-d.*(commit|pushed|comment|commented)_at.*")
262265
)
263266
}
264267
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,8 @@ class EnvPathInjectionFromFileReadSink extends EnvPathInjectionSink {
3737
// e.g.
3838
// FOO=$(cat test-results/sha-number)
3939
// echo "FOO=$FOO" >> $GITHUB_PATH
40-
exists(string line, string var_name, string var_value |
41-
run.getScript().splitAt("\n") = line
42-
|
43-
var_name = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 1) and
44-
var_value = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 2) and
40+
exists(string var_name, string var_value |
41+
run.getAnAssignment(var_name, var_value) and
4542
outputsPartialFileContent(var_value) and
4643
(
4744
value.matches("%$" + ["", "{", "ENV{"] + var_name + "%")

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,8 @@ class EnvVarInjectionFromFileReadSink extends EnvVarInjectionSink {
4242
// e.g.
4343
// FOO=$(cat test-results/sha-number)
4444
// echo "FOO=$FOO" >> $GITHUB_ENV
45-
exists(string line, string var_name, string var_value |
46-
run.getScript().splitAt("\n") = line
47-
|
48-
var_name = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 1) and
49-
var_value = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 2) and
45+
exists(string var_name, string var_value |
46+
run.getAnAssignment(var_name, var_value) and
5047
outputsPartialFileContent(var_value) and
5148
(
5249
value.matches("%$" + ["", "{", "ENV{"] + var_name + "%")

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,8 @@ class OutputClobberingFromFileReadSink extends OutputClobberingSink {
5656
// e.g.
5757
// FOO=$(cat test-results/sha-number)
5858
// echo "FOO=$FOO" >> $GITHUB_OUTPUT
59-
exists(string line, string var_name, string var_value |
60-
run.getScript().splitAt("\n") = line
61-
|
62-
var_name = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 1) and
63-
var_value = line.regexpCapture("([a-zA-Z0-9\\-_]+)=(.*)", 2) and
59+
exists(string var_name, string var_value |
60+
run.getAnAssignment(var_name, var_value) and
6461
outputsPartialFileContent(var_value) and
6562
(
6663
value.matches("%$" + ["", "{", "ENV{"] + var_name + "%")
@@ -154,11 +151,11 @@ class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
154151
// A file is read and its content is printed to stdout
155152
// - run: echo "foo=$(<pr-id.txt)"
156153
clobbering_line.regexpMatch(".*echo\\s+(-e)?\\s*(\"|')?") and
157-
clobbering_line.regexpMatch(partialFileContentRegexp() + ".*")
154+
clobbering_line.regexpMatch(["ls", Bash::partialFileContentCommand()] + "\\s.*")
158155
or
159156
// A file content is printed to stdout
160157
// - run: cat pr-id.txt
161-
clobbering_line.regexpMatch(partialFileContentRegexp() + ".*")
158+
clobbering_line.regexpMatch(["ls", Bash::partialFileContentCommand()] + "\\s.*")
162159
)
163160
)
164161
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class PoisonableCommandStep extends PoisonableStep, Run {
1818
PoisonableCommandStep() {
1919
exists(string regexp |
2020
poisonableCommandsDataModel(regexp) and
21-
exists(this.getScript().splitAt("\n").trim().regexpFind(regexp, _, _))
21+
exists(this.getACommand().regexpFind(regexp, _, _))
2222
)
2323
}
2424
}
@@ -39,11 +39,9 @@ class LocalScriptExecutionRunStep extends PoisonableStep, Run {
3939
string path;
4040

4141
LocalScriptExecutionRunStep() {
42-
exists(string line, string regexp, int path_group |
43-
line = this.getScript().splitAt("\n").trim()
44-
|
42+
exists(string cmd, string regexp, int path_group | cmd = this.getACommand() |
4543
poisonableLocalScriptsDataModel(regexp, path_group) and
46-
path = line.regexpCapture(regexp, path_group)
44+
path = cmd.regexpCapture(regexp, path_group)
4745
)
4846
}
4947

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

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -265,19 +265,18 @@ class ActionsSHACheckout extends SHACheckoutStep instanceof UsesStep {
265265
/** Checkout of a Pull Request HEAD ref using git within a Run step */
266266
class GitMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
267267
GitMutableRefCheckout() {
268-
exists(string line |
269-
this.getScript().splitAt("\n") = line and
270-
line.regexpMatch(".*git\\s+(fetch|pull).*") and
268+
exists(string cmd | this.getACommand() = cmd |
269+
cmd.regexpMatch("git\\s+(fetch|pull).*") and
271270
(
272-
(containsHeadRef(line) or containsPullRequestNumber(line))
271+
(containsHeadRef(cmd) or containsPullRequestNumber(cmd))
273272
or
274273
exists(string varname, string expr |
275274
expr = this.getInScopeEnvVarExpr(varname).getExpression() and
276275
(
277276
containsHeadRef(expr) or
278277
containsPullRequestNumber(expr)
279278
) and
280-
exists(line.regexpFind(varname, _, _))
279+
exists(cmd.regexpFind(varname, _, _))
281280
)
282281
)
283282
)
@@ -289,16 +288,15 @@ class GitMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
289288
/** Checkout of a Pull Request HEAD ref using git within a Run step */
290289
class GitSHACheckout extends SHACheckoutStep instanceof Run {
291290
GitSHACheckout() {
292-
exists(string line |
293-
this.getScript().splitAt("\n") = line and
294-
line.regexpMatch(".*git\\s+(fetch|pull).*") and
291+
exists(string cmd | this.getACommand() = cmd |
292+
cmd.regexpMatch("git\\s+(fetch|pull).*") and
295293
(
296-
containsHeadSHA(line)
294+
containsHeadSHA(cmd)
297295
or
298296
exists(string varname, string expr |
299297
expr = this.getInScopeEnvVarExpr(varname).getExpression() and
300298
containsHeadSHA(expr) and
301-
exists(line.regexpFind(varname, _, _))
299+
exists(cmd.regexpFind(varname, _, _))
302300
)
303301
)
304302
)
@@ -310,18 +308,17 @@ class GitSHACheckout extends SHACheckoutStep instanceof Run {
310308
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
311309
class GhMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
312310
GhMutableRefCheckout() {
313-
exists(string line |
314-
this.getScript().splitAt("\n") = line and
315-
line.regexpMatch(".*(gh|hub)\\s+pr\\s+checkout.*") and
311+
exists(string cmd | this.getACommand() = cmd |
312+
cmd.regexpMatch(".*(gh|hub)\\s+pr\\s+checkout.*") and
316313
(
317-
(containsHeadRef(line) or containsPullRequestNumber(line))
314+
(containsHeadRef(cmd) or containsPullRequestNumber(cmd))
318315
or
319316
exists(string varname |
320317
(
321318
containsHeadRef(this.getInScopeEnvVarExpr(varname).getExpression()) or
322319
containsPullRequestNumber(this.getInScopeEnvVarExpr(varname).getExpression())
323320
) and
324-
exists(line.regexpFind(varname, _, _))
321+
exists(cmd.regexpFind(varname, _, _))
325322
)
326323
)
327324
)
@@ -333,15 +330,14 @@ class GhMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
333330
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
334331
class GhSHACheckout extends SHACheckoutStep instanceof Run {
335332
GhSHACheckout() {
336-
exists(string line |
337-
this.getScript().splitAt("\n") = line and
338-
line.regexpMatch(".*gh\\s+pr\\s+checkout.*") and
333+
exists(string cmd | this.getACommand() = cmd |
334+
cmd.regexpMatch("gh\\s+pr\\s+checkout.*") and
339335
(
340-
containsHeadSHA(line)
336+
containsHeadSHA(cmd)
341337
or
342338
exists(string varname |
343339
containsHeadSHA(this.getInScopeEnvVarExpr(varname).getExpression()) and
344-
exists(line.regexpFind(varname, _, _))
340+
exists(cmd.regexpFind(varname, _, _))
345341
)
346342
)
347343
)

0 commit comments

Comments
 (0)