Skip to content

Commit e2e1ddd

Browse files
author
Alvaro Muñoz
committed
Move arg injection sinks to ShellScript class
1 parent 2e5379f commit e2e1ddd

8 files changed

Lines changed: 139 additions & 96 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,14 @@ class ShellScript extends ScalarValueImpl instanceof ShellScriptImpl {
354354

355355
predicate getACmdReachingGitHubPathWrite(string cmd) { super.getACmdReachingGitHubPathWrite(cmd) }
356356

357+
predicate getAnEnvReachingArgumentInjectionSink(string var, string command, string argument) {
358+
super.getAnEnvReachingArgumentInjectionSink(var, command, argument)
359+
}
360+
361+
predicate getACmdReachingArgumentInjectionSink(string cmd, string command, string argument) {
362+
super.getACmdReachingArgumentInjectionSink(cmd, command, argument)
363+
}
364+
357365
predicate fileToGitHubEnv(string path) { super.fileToGitHubEnv(path) }
358366

359367
predicate fileToGitHubOutput(string path) { super.fileToGitHubOutput(path) }

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

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ class BashShellScript extends ShellScript {
133133
this.doStmtRestoreQuotedStrings(i, round, _, new)
134134
|
135135
new order by round
136-
)
136+
) and
137+
not result.indexOf("qstr:") > -1
137138
}
138139

139140
private predicate doStmtRestoreCmdSubstitutions(int line, int round, string old, string new) {
@@ -155,7 +156,8 @@ class BashShellScript extends ShellScript {
155156
this.doStmtRestoreCmdSubstitutions(i, round, _, new)
156157
|
157158
new order by round
158-
)
159+
) and
160+
not result.indexOf("cmdsubs:") > -1
159161
}
160162

161163
override string getAStmt() { result = this.getStmt(_) }
@@ -186,7 +188,8 @@ class BashShellScript extends ShellScript {
186188
this.doCmdRestoreQuotedStrings(i, round, _, new)
187189
|
188190
new order by round
189-
)
191+
) and
192+
not result.indexOf("qstr:") > -1
190193
}
191194

192195
private predicate doCmdRestoreCmdSubstitutions(int line, int round, string old, string new) {
@@ -208,13 +211,16 @@ class BashShellScript extends ShellScript {
208211
this.doCmdRestoreCmdSubstitutions(i, round, _, new)
209212
|
210213
new order by round
211-
)
214+
) and
215+
not result.indexOf("cmdsubs:") > -1
212216
}
213217

214218
string getACmd() { result = this.getCmd(_) }
215219

216220
override string getCommand(int i) {
217-
result = this.getCmd(i) and
221+
// remove redirection
222+
result =
223+
this.getCmd(i).regexpReplaceAll("(>|>>|2>|2>>|<|<<<)\\s*[\\{\\}\\$\"'_\\-0-9a-zA-Z]+$", "") and
218224
// exclude variable declarations
219225
not result.regexpMatch("^[a-zA-Z0-9\\-_]+=") and
220226
// exclude the following keywords
@@ -286,6 +292,18 @@ class BashShellScript extends ShellScript {
286292
Bash::cmdReachingGitHubFileWrite(this, cmd, "GITHUB_PATH", _)
287293
}
288294

295+
override predicate getAnEnvReachingArgumentInjectionSink(
296+
string var, string command, string argument
297+
) {
298+
Bash::envReachingArgumentInjectionSink(this, var, command, argument)
299+
}
300+
301+
override predicate getACmdReachingArgumentInjectionSink(
302+
string cmd, string command, string argument
303+
) {
304+
Bash::cmdReachingArgumentInjectionSink(this, cmd, command, argument)
305+
}
306+
289307
override predicate fileToGitHubEnv(string path) {
290308
Bash::fileToFileWrite(this, "GITHUB_ENV", path)
291309
}
@@ -633,6 +651,30 @@ module Bash {
633651
)
634652
}
635653

654+
predicate envReachingArgumentInjectionSink(
655+
BashShellScript script, string source, string command, string argument
656+
) {
657+
exists(string cmd, string regex, int command_group, int argument_group |
658+
cmd = script.getACommand() and
659+
argumentInjectionSinksDataModel(regex, command_group, argument_group) and
660+
argument = cmd.regexpCapture(regex, argument_group) and
661+
command = cmd.regexpCapture(regex, command_group) and
662+
envReachingRunExpr(script, source, argument)
663+
)
664+
}
665+
666+
predicate cmdReachingArgumentInjectionSink(
667+
BashShellScript script, string source, string command, string argument
668+
) {
669+
exists(string cmd, string regex, int command_group, int argument_group |
670+
cmd = script.getACommand() and
671+
argumentInjectionSinksDataModel(regex, command_group, argument_group) and
672+
argument = cmd.regexpCapture(regex, argument_group) and
673+
command = cmd.regexpCapture(regex, command_group) and
674+
cmdReachingRunExpr(script, source, argument)
675+
)
676+
}
677+
636678
/**
637679
* Holds if a command output is used, directly or indirectly, in a Run's step expression.
638680
* Where the expression is a string captured from the Run's script.

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,18 @@ class PowerShellScript extends ShellScript {
4242

4343
override predicate getACmdReachingGitHubPathWrite(string cmd) { none() }
4444

45+
override predicate getAnEnvReachingArgumentInjectionSink(
46+
string var, string command, string argument
47+
) {
48+
none()
49+
}
50+
51+
override predicate getACmdReachingArgumentInjectionSink(
52+
string cmd, string command, string argument
53+
) {
54+
none()
55+
}
56+
4557
override predicate fileToGitHubEnv(string path) { none() }
4658

4759
override predicate fileToGitHubOutput(string path) { none() }

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,14 @@ class ShellScriptImpl extends ScalarValueImpl {
227227

228228
abstract predicate getACmdReachingGitHubPathWrite(string cmd);
229229

230+
abstract predicate getAnEnvReachingArgumentInjectionSink(
231+
string var, string command, string argument
232+
);
233+
234+
abstract predicate getACmdReachingArgumentInjectionSink(
235+
string cmd, string command, string argument
236+
);
237+
230238
abstract predicate fileToGitHubEnv(string path);
231239

232240
abstract predicate fileToGitHubOutput(string path);

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

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,6 @@ abstract class ArgumentInjectionSink extends DataFlow::Node {
99
abstract string getCommand();
1010
}
1111

12-
/**
13-
* Holds if an environment variable is used, directly or indirectly, as an argument to a dangerous command
14-
* in a Run step.
15-
* Where the command is a string captured from the Run's script.
16-
*/
17-
bindingset[var]
18-
predicate envToArgInjSink(string var, Run run, string command) {
19-
exists(string argument, string cmd, string regexp, int command_group, int argument_group |
20-
run.getScript().getACommand() = cmd and
21-
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
22-
command = cmd.regexpCapture(regexp, command_group) and
23-
argument = cmd.regexpCapture(regexp, argument_group) and
24-
Bash::envReachingRunExpr(run.getScript(), var, argument) and
25-
exists(run.getInScopeEnvVarExpr(var))
26-
)
27-
}
28-
2912
/**
3013
* Holds if a Run step declares an environment variable, uses it as the argument to a command vulnerable to argument injection.
3114
* e.g.
@@ -36,23 +19,16 @@ predicate envToArgInjSink(string var, Run run, string command) {
3619
*/
3720
class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink {
3821
string command;
22+
string argument;
3923

4024
ArgumentInjectionFromEnvVarSink() {
4125
exists(Run run, string var |
42-
envToArgInjSink(var, run, command) and
43-
run.getScript() = this.asExpr() and
44-
exists(run.getInScopeEnvVarExpr(var))
45-
)
46-
or
47-
exists(
48-
Run run, string cmd, string argument, string regexp, int argument_group, int command_group
49-
|
50-
run.getScript().getACommand() = cmd and
5126
run.getScript() = this.asExpr() and
52-
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
53-
argument = cmd.regexpCapture(regexp, argument_group) and
54-
command = cmd.regexpCapture(regexp, command_group) and
55-
argument.regexpMatch(".*\\$(\\{)?(GITHUB_HEAD_REF).*")
27+
(
28+
exists(run.getInScopeEnvVarExpr(var)) or
29+
var = "GITHUB_HEAD_REF"
30+
) and
31+
run.getScript().getAnEnvReachingArgumentInjectionSink(var, command, argument)
5632
)
5733
}
5834

@@ -68,18 +44,13 @@ class ArgumentInjectionFromEnvVarSink extends ArgumentInjectionSink {
6844
*/
6945
class ArgumentInjectionFromCommandSink extends ArgumentInjectionSink {
7046
string command;
47+
string argument;
7148

7249
ArgumentInjectionFromCommandSink() {
73-
exists(
74-
CommandSource source, Run run, string cmd, string argument, string regexp, int argument_group,
75-
int command_group
76-
|
50+
exists(CommandSource source, Run run |
7751
run = source.getEnclosingRun() and
7852
this.asExpr() = run.getScript() and
79-
cmd = run.getScript().getACommand() and
80-
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
81-
argument = cmd.regexpCapture(regexp, argument_group) and
82-
command = cmd.regexpCapture(regexp, command_group)
53+
run.getScript().getACmdReachingArgumentInjectionSink(source.getCommand(), command, argument)
8354
)
8455
}
8556

@@ -103,14 +74,9 @@ private module ArgumentInjectionConfig implements DataFlow::ConfigSig {
10374
predicate isSource(DataFlow::Node source) {
10475
source instanceof RemoteFlowSource
10576
or
106-
exists(
107-
Run run, string argument, string cmd, string regexp, int command_group, int argument_group
108-
|
77+
exists(Run run |
10978
run.getScript() = source.asExpr() and
110-
run.getScript().getACommand() = cmd and
111-
argumentInjectionSinksDataModel(regexp, command_group, argument_group) and
112-
argument = cmd.regexpCapture(regexp, argument_group) and
113-
argument.regexpMatch(".*\\$(\\{)?(GITHUB_HEAD_REF).*")
79+
run.getScript().getAnEnvReachingArgumentInjectionSink("GITHUB_HEAD_REF", _, _)
11480
)
11581
}
11682

@@ -120,7 +86,7 @@ private module ArgumentInjectionConfig implements DataFlow::ConfigSig {
12086
exists(Run run, string var |
12187
run.getInScopeEnvVarExpr(var) = pred.asExpr() and
12288
succ.asExpr() = run.getScript() and
123-
envToArgInjSink(var, run, _)
89+
run.getScript().getAnEnvReachingArgumentInjectionSink(var, _, _)
12490
)
12591
}
12692
}

0 commit comments

Comments
 (0)