Skip to content

Commit 78f0ba2

Browse files
owen-mcCopilot
andcommitted
Detect commands passed through simple launchers
Recognize fixed command positions without parsing launcher options. Only sanitize later arguments when the delegated command is one unchanged string literal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 88ef6cf commit 78f0ba2

5 files changed

Lines changed: 401 additions & 66 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `java/command-line-injection` query now detects command injection through simple invocations of delegating launchers such as `env`, `sudo`, and `timeout`.

‎java/ql/lib/semmle/code/java/JDK.qll‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ class StringPartialMatchMethod extends Method {
104104
}
105105
}
106106

107+
/** The type `String[]`. */
108+
class ArrayOfStringType extends Array {
109+
ArrayOfStringType() {
110+
this.getDimension() = 1 and
111+
this.getElementType() instanceof TypeString
112+
}
113+
}
114+
107115
/** The class `java.lang.StringBuffer`. */
108116
class TypeStringBuffer extends Class {
109117
TypeStringBuffer() { this.hasQualifiedName("java.lang", "StringBuffer") }
@@ -444,11 +452,7 @@ class MainMethod extends Method {
444452
this.getReturnType().hasName("void") and
445453
this.hasName("main") and
446454
this.getNumberOfParameters() = 1 and
447-
exists(Array a |
448-
a = this.getAParameter().getType() and
449-
a.getDimension() = 1 and
450-
a.getElementType() instanceof TypeString
451-
)
455+
this.getAParameter().getType() instanceof ArrayOfStringType
452456
}
453457
}
454458

‎java/ql/lib/semmle/code/java/security/CommandArguments.qll‎

Lines changed: 193 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,46 +10,152 @@ import semmle.code.java.Collections
1010

1111
/**
1212
* Holds if `ex` is used safely as an argument to a command;
13-
* i.e. it's not in the first position and it's not a shell command.
13+
* i.e. it's not in a position that selects a command to execute.
1414
*/
1515
predicate isSafeCommandArgument(Expr ex) {
16-
exists(ArrayInit ai, int i |
17-
ex = ai.getInit(i) and
18-
i > 0 and
19-
not isShell(ai.getInit(0))
16+
exists(ArrayInit init, int index |
17+
ex = init.getInit(index) and
18+
index > 0 and
19+
isNonDelegatingCommand(init.getInit(0))
2020
)
2121
or
22-
exists(CommandArgumentList cal |
23-
not cal.isShell() and
24-
ex = cal.getASubsequentAdd().getArgument(0)
22+
exists(CommandArgumentList list |
23+
not list.isShell() and
24+
not list.isDelegatingLauncher() and
25+
ex = list.getASubsequentAdd().getArgument(0)
2526
)
2627
or
27-
exists(CommandArgArrayImmutableFirst caa |
28-
not caa.isShell() and
29-
ex = caa.getAWrite(any(int i | i > 0))
28+
exists(CommandArgArrayImmutableFirst array |
29+
not array.isShell() and
30+
not array.isDelegatingLauncher() and
31+
ex = array.getAWrite(any(int index | index > 0))
32+
)
33+
or
34+
exists(CommandArguments arguments, int i |
35+
ex = arguments.getArgument(i) and
36+
forall(CommandArguments otherArguments, int otherIndex |
37+
ex = otherArguments.getArgument(otherIndex)
38+
|
39+
otherIndex > 0 and otherArguments.hasSafeDelegatedArgument(otherIndex)
40+
)
3041
)
3142
}
3243

33-
/**
34-
* Holds if the given expression is the name of a shell command such as bash or python
35-
*/
36-
private predicate isShell(Expr ex) {
37-
exists(string cmd | cmd = ex.(StringLiteral).getValue() |
38-
cmd.regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
39-
)
44+
private string commandName(Expr ex) {
45+
result = ex.(StringLiteral).getValue()
4046
or
4147
exists(SsaDefinition ssa, SsaExplicitWrite def |
4248
ex = ssa.getARead() and
43-
isShell(def.getDefiningExpr())
49+
result = commandName(def.getDefiningExpr())
4450
|
4551
ssa.getAnUltimateDefinition() = def
4652
or
4753
ssa.(SsaCapturedDefinition).getAnUltimateCapturedDefinition() = def
4854
)
4955
or
50-
isShell(ex.(Assignment).getRhs())
56+
result = commandName(ex.(Assignment).getRhs())
5157
or
52-
isShell(ex.(LocalVariableDeclExpr).getInit())
58+
result = commandName(ex.(LocalVariableDeclExpr).getInit())
59+
}
60+
61+
/**
62+
* Holds if the given expression is the name of a shell command such as bash or python
63+
*/
64+
private predicate isShell(Expr ex) {
65+
commandName(ex).regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
66+
}
67+
68+
/**
69+
* Gets the fixed position of the command executed by a supported delegating launcher.
70+
*
71+
* Only simple invocation forms with a command at the expected position are recognized. Launcher
72+
* options and environment assignments are intentionally not interpreted.
73+
*/
74+
private int delegatedCommandIndex(Expr ex) {
75+
exists(string launcher |
76+
launcher = commandName(ex).regexpCapture("(.*/)?([^/.]+)(\\.exe)?$", 2) and
77+
(
78+
launcher = ["env", "nice", "nohup", "setsid", "sudo", "xargs"] and result = 1
79+
or
80+
launcher = ["chroot", "stdbuf", "timeout"] and result = 2
81+
)
82+
)
83+
}
84+
85+
private predicate isNonDelegatingCommand(Expr ex) {
86+
exists(commandName(ex)) and
87+
not isShell(ex) and
88+
not exists(delegatedCommandIndex(ex))
89+
}
90+
91+
/** Holds if `init` initializes an array passed directly to a call. */
92+
private predicate isInlineArgumentArray(ArrayInit init) {
93+
exists(ArrayCreationExpr array, Call call |
94+
array.getInit() = init and
95+
array = call.getAnArgument() and
96+
not exists(MethodCall methodCall |
97+
call = methodCall and
98+
methodCall.getMethod().getDeclaringType().hasQualifiedName("java.util", "Arrays") and
99+
methodCall.getMethod().hasName("copyOf")
100+
)
101+
)
102+
}
103+
104+
private newtype TCommandArguments =
105+
TLiteralArguments(ArrayInit init) { isInlineArgumentArray(init) } or
106+
TListArguments(CommandArgumentList list) or
107+
TMutableArguments(CommandArgArrayImmutableFirst array)
108+
109+
/** An ordered sequence of command arguments. */
110+
private class CommandArguments extends TCommandArguments {
111+
string toString() { result = this.getArgument(0).toString() }
112+
113+
/** Gets a possible value at the given index. */
114+
Expr getArgument(int index) {
115+
exists(ArrayInit init |
116+
this = TLiteralArguments(init) and
117+
result = init.getInit(index)
118+
)
119+
or
120+
exists(CommandArgumentList list |
121+
this = TListArguments(list) and
122+
(
123+
result = list.getArgument(index)
124+
or
125+
index = 3 and result = list.getAnArgumentAfterKnownPrefix().getArgument(0)
126+
)
127+
)
128+
or
129+
exists(CommandArgArrayImmutableFirst array |
130+
this = TMutableArguments(array) and
131+
result = array.getAnElement(index)
132+
)
133+
}
134+
135+
/** Gets the single string literal stored at the given index, if there is one. */
136+
private StringLiteral getFixedLiteralArgument(int index) {
137+
result = this.getArgument(index) and
138+
forall(Expr value | value = this.getArgument(index) | value = result)
139+
}
140+
141+
predicate hasSafeDelegatedArgument(int index) {
142+
exists(this.getArgument(index)) and
143+
forall(Expr launcher | launcher = this.getArgument(0) |
144+
exists(delegatedCommandIndex(launcher)) and
145+
forall(int commandIndex | commandIndex = delegatedCommandIndex(launcher) |
146+
(
147+
index < commandIndex
148+
or
149+
index > commandIndex and
150+
exists(StringLiteral command |
151+
command = this.getFixedLiteralArgument(commandIndex) and
152+
not isShell(command) and
153+
not exists(delegatedCommandIndex(command))
154+
)
155+
)
156+
)
157+
)
158+
}
53159
}
54160

55161
/**
@@ -101,6 +207,39 @@ private class CommandArgumentList extends SsaExplicitWrite {
101207
result.getQualifier() = this.getAUseBeforeFirstAdd()
102208
}
103209

210+
/** Gets a single-element addition at the given index. */
211+
private MethodCall getAnAddAtIndex(int index) {
212+
index = 0 and
213+
result = this.getAFirstAdd() and
214+
result.getMethod().hasName("add") and
215+
result.getNumArgument() = 1
216+
or
217+
index > 0 and
218+
index <= 2 and
219+
exists(MethodCall previous |
220+
previous = this.getAnAddAtIndex(index - 1) and
221+
result = this.getAnAdd() and
222+
result.getMethod().hasName("add") and
223+
result.getNumArgument() = 1 and
224+
adjacentUseUse(previous.getQualifier(), result.getQualifier())
225+
)
226+
}
227+
228+
/** Gets a single-element addition after the first three elements. */
229+
MethodCall getAnArgumentAfterKnownPrefix() {
230+
result = this.getAnAdd() and
231+
result.getMethod().hasName("add") and
232+
result.getNumArgument() = 1 and
233+
exists(MethodCall previous |
234+
previous = this.getAnAddAtIndex(2) or previous = this.getAnArgumentAfterKnownPrefix()
235+
|
236+
adjacentUseUse(previous.getQualifier(), result.getQualifier())
237+
)
238+
}
239+
240+
/** Gets the argument added at the given index. */
241+
Expr getArgument(int index) { result = this.getAnAddAtIndex(index).getArgument(0) }
242+
104243
/** Gets an addition to this list which is not the first element. */
105244
MethodCall getASubsequentAdd() {
106245
result = this.getAnAdd() and
@@ -109,15 +248,15 @@ private class CommandArgumentList extends SsaExplicitWrite {
109248

110249
/** Holds if the first element of this list is a shell command. */
111250
predicate isShell() {
112-
exists(MethodCall ma | ma = this.getAFirstAdd() and isShell(ma.getArgument(0)))
251+
exists(MethodCall add | add = this.getAFirstAdd() and isShell(add.getArgument(0)))
113252
}
114-
}
115253

116-
/**
117-
* The type `String[]`.
118-
*/
119-
private class ArrayOfStringType extends Array {
120-
ArrayOfStringType() { this.getElementType() instanceof TypeString }
254+
/** Holds if the first element of this list is a delegating launcher. */
255+
predicate isDelegatingLauncher() {
256+
exists(MethodCall add | add = this.getAFirstAdd() |
257+
exists(delegatedCommandIndex(add.getArgument(0)))
258+
)
259+
}
121260
}
122261

123262
private predicate arrayVarWrite(ArrayAccess acc) { exists(Assignment a | a.getDest() = acc) }
@@ -153,7 +292,7 @@ private class CommandArgumentArray extends SsaExplicitWrite {
153292
*/
154293
private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
155294
CommandArgArrayImmutableFirst() {
156-
(exists(this.getAWrite(0)) or exists(firstElementOf(this.getDefiningExpr()))) and
295+
(exists(this.getAWrite(0)) or exists(arrayElementOf(this.getDefiningExpr(), 0))) and
157296
forall(VarRead use | exists(this.getAWrite(0, use)) | use = ssaGetAFirstUse(this))
158297
}
159298

@@ -162,41 +301,56 @@ private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
162301
result = this.getAWrite(0)
163302
or
164303
not exists(this.getAWrite(0)) and
165-
result = firstElementOf(this.getDefiningExpr())
304+
result = arrayElementOf(this.getDefiningExpr(), 0)
166305
}
167306

168307
/** Holds if the first element of this array is a shell command. */
169308
predicate isShell() { isShell(this.getFirstElement()) }
309+
310+
/** Holds if the first element of this array is a delegating launcher. */
311+
predicate isDelegatingLauncher() { exists(delegatedCommandIndex(this.getFirstElement())) }
312+
313+
/** Gets a possible value for the element at the given index. */
314+
Expr getAnElement(int index) {
315+
index = 0 and result = this.getFirstElement()
316+
or
317+
index > 0 and
318+
(
319+
result = this.getAWrite(index)
320+
or
321+
result = arrayElementOf(this.getDefiningExpr(), index)
322+
)
323+
}
170324
}
171325

172-
/** Gets the first element of an imutable array of strings */
173-
private Expr firstElementOf(Expr arr) {
326+
/** Gets an element of an immutable array of strings. */
327+
private Expr arrayElementOf(Expr arr, int index) {
174328
arr.getType() instanceof ArrayOfStringType and
175329
(
176-
result = firstElementOf(arr.(Assignment).getRhs())
330+
result = arrayElementOf(arr.(Assignment).getRhs(), index)
177331
or
178-
result = firstElementOf(arr.(LocalVariableDeclExpr).getInit())
332+
result = arrayElementOf(arr.(LocalVariableDeclExpr).getInit(), index)
179333
or
180334
exists(CommandArgArrayImmutableFirst caa | arr = caa.getARead() |
181-
result = caa.getFirstElement()
335+
result = caa.getAnElement(index)
182336
)
183337
or
184338
exists(MethodCall ma, Method m |
185339
arr = ma and
186340
ma.getMethod() = m and
187341
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
188342
m.hasName("copyOf") and
189-
result = firstElementOf(ma.getArgument(0))
343+
result = arrayElementOf(ma.getArgument(0), index)
190344
)
191345
or
192346
exists(Field f |
193347
f.isStatic() and
194348
arr.(FieldRead).getField() = f and
195-
result = firstElementOf(f.getInitializer())
349+
result = arrayElementOf(f.getInitializer(), index)
196350
)
197351
or
198-
result = arr.(ArrayInit).getInit(0)
352+
result = arr.(ArrayInit).getInit(index)
199353
or
200-
result = arr.(ArrayCreationExpr).getInit().getInit(0)
354+
result = arr.(ArrayCreationExpr).getInit().getInit(index)
201355
)
202356
}

0 commit comments

Comments
 (0)