Skip to content

Commit 8f59bbb

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 8f59bbb

5 files changed

Lines changed: 394 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: 186 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,46 +10,145 @@ 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+
forex(CommandArguments arguments, int index | ex = arguments.getArgument(index) |
35+
index > 0 and arguments.hasSafeDelegatedArgument(index)
3036
)
3137
}
3238

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

55154
/**
@@ -101,6 +200,39 @@ private class CommandArgumentList extends SsaExplicitWrite {
101200
result.getQualifier() = this.getAUseBeforeFirstAdd()
102201
}
103202

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

110242
/** Holds if the first element of this list is a shell command. */
111243
predicate isShell() {
112-
exists(MethodCall ma | ma = this.getAFirstAdd() and isShell(ma.getArgument(0)))
244+
exists(MethodCall add | add = this.getAFirstAdd() and isShell(add.getArgument(0)))
113245
}
114-
}
115246

116-
/**
117-
* The type `String[]`.
118-
*/
119-
private class ArrayOfStringType extends Array {
120-
ArrayOfStringType() { this.getElementType() instanceof TypeString }
247+
/** Holds if the first element of this list is a delegating launcher. */
248+
predicate isDelegatingLauncher() {
249+
exists(MethodCall add | add = this.getAFirstAdd() |
250+
exists(delegatedCommandIndex(add.getArgument(0)))
251+
)
252+
}
121253
}
122254

123255
private predicate arrayVarWrite(ArrayAccess acc) { exists(Assignment a | a.getDest() = acc) }
@@ -153,7 +285,7 @@ private class CommandArgumentArray extends SsaExplicitWrite {
153285
*/
154286
private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
155287
CommandArgArrayImmutableFirst() {
156-
(exists(this.getAWrite(0)) or exists(firstElementOf(this.getDefiningExpr()))) and
288+
(exists(this.getAWrite(0)) or exists(arrayElementOf(this.getDefiningExpr(), 0))) and
157289
forall(VarRead use | exists(this.getAWrite(0, use)) | use = ssaGetAFirstUse(this))
158290
}
159291

@@ -162,41 +294,56 @@ private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
162294
result = this.getAWrite(0)
163295
or
164296
not exists(this.getAWrite(0)) and
165-
result = firstElementOf(this.getDefiningExpr())
297+
result = arrayElementOf(this.getDefiningExpr(), 0)
166298
}
167299

168300
/** Holds if the first element of this array is a shell command. */
169301
predicate isShell() { isShell(this.getFirstElement()) }
302+
303+
/** Holds if the first element of this array is a delegating launcher. */
304+
predicate isDelegatingLauncher() { exists(delegatedCommandIndex(this.getFirstElement())) }
305+
306+
/** Gets a possible value for the element at the given index. */
307+
Expr getAnElement(int index) {
308+
index = 0 and result = this.getFirstElement()
309+
or
310+
index > 0 and
311+
(
312+
result = this.getAWrite(index)
313+
or
314+
result = arrayElementOf(this.getDefiningExpr(), index)
315+
)
316+
}
170317
}
171318

172-
/** Gets the first element of an imutable array of strings */
173-
private Expr firstElementOf(Expr arr) {
319+
/** Gets an element of an immutable array of strings. */
320+
private Expr arrayElementOf(Expr arr, int index) {
174321
arr.getType() instanceof ArrayOfStringType and
175322
(
176-
result = firstElementOf(arr.(Assignment).getRhs())
323+
result = arrayElementOf(arr.(Assignment).getRhs(), index)
177324
or
178-
result = firstElementOf(arr.(LocalVariableDeclExpr).getInit())
325+
result = arrayElementOf(arr.(LocalVariableDeclExpr).getInit(), index)
179326
or
180327
exists(CommandArgArrayImmutableFirst caa | arr = caa.getARead() |
181-
result = caa.getFirstElement()
328+
result = caa.getAnElement(index)
182329
)
183330
or
184331
exists(MethodCall ma, Method m |
185332
arr = ma and
186333
ma.getMethod() = m and
187334
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
188335
m.hasName("copyOf") and
189-
result = firstElementOf(ma.getArgument(0))
336+
result = arrayElementOf(ma.getArgument(0), index)
190337
)
191338
or
192339
exists(Field f |
193340
f.isStatic() and
194341
arr.(FieldRead).getField() = f and
195-
result = firstElementOf(f.getInitializer())
342+
result = arrayElementOf(f.getInitializer(), index)
196343
)
197344
or
198-
result = arr.(ArrayInit).getInit(0)
345+
result = arr.(ArrayInit).getInit(index)
199346
or
200-
result = arr.(ArrayCreationExpr).getInit().getInit(0)
347+
result = arr.(ArrayCreationExpr).getInit().getInit(index)
201348
)
202349
}

0 commit comments

Comments
 (0)