Skip to content

Commit f1d360c

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 59b4016 commit f1d360c

4 files changed

Lines changed: 278 additions & 71 deletions

File tree

‎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: 137 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -10,46 +10,120 @@ 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
16+
exists(CommandArguments arguments, int i |
1817
i > 0 and
19-
not isShell(ai.getInit(0))
20-
)
21-
or
22-
exists(CommandArgumentList cal |
23-
not cal.isShell() and
24-
ex = cal.getASubsequentAdd().getArgument(0)
25-
)
26-
or
27-
exists(CommandArgArrayImmutableFirst caa |
28-
not caa.isShell() and
29-
ex = caa.getAWrite(any(int i | i > 0))
18+
ex = arguments.getArgument(i) and
19+
arguments.hasSafeArgument(i)
3020
)
3121
}
3222

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-
)
23+
private string commandName(Expr ex) {
24+
result = ex.(StringLiteral).getValue()
4025
or
4126
exists(SsaDefinition ssa, SsaExplicitWrite def |
4227
ex = ssa.getARead() and
43-
isShell(def.getDefiningExpr())
28+
result = commandName(def.getDefiningExpr())
4429
|
4530
ssa.getAnUltimateDefinition() = def
4631
or
4732
ssa.(SsaCapturedDefinition).getAnUltimateCapturedDefinition() = def
4833
)
4934
or
50-
isShell(ex.(Assignment).getRhs())
35+
result = commandName(ex.(Assignment).getRhs())
5136
or
52-
isShell(ex.(LocalVariableDeclExpr).getInit())
37+
result = commandName(ex.(LocalVariableDeclExpr).getInit())
38+
}
39+
40+
/**
41+
* Holds if the given expression is the name of a shell command such as bash or python
42+
*/
43+
private predicate isShell(Expr ex) {
44+
commandName(ex).regexpMatch(".*(sh|javac?|python[23]?|osascript|cmd)(\\.exe)?$")
45+
}
46+
47+
/**
48+
* Gets the fixed position of the command executed by a supported delegating launcher.
49+
*
50+
* Only simple invocation forms with a command at the expected position are recognized. Launcher
51+
* options and environment assignments are intentionally not interpreted.
52+
*/
53+
private int delegatedCommandIndex(Expr ex) {
54+
exists(string launcher |
55+
launcher = commandName(ex).regexpCapture("(.*/)?([^/.]+)(\\.exe)?$", 2) and
56+
(
57+
launcher = ["env", "nice", "nohup", "setsid", "sudo", "xargs"] and result = 1
58+
or
59+
launcher = ["chroot", "stdbuf", "timeout"] and result = 2
60+
)
61+
)
62+
}
63+
64+
/** Holds if `init` initializes an array passed directly to a call. */
65+
private predicate isInlineArgumentArray(ArrayInit init) {
66+
exists(ArrayCreationExpr array, Call call |
67+
array.getInit() = init and
68+
array = call.getAnArgument()
69+
)
70+
}
71+
72+
private newtype TCommandArguments =
73+
TLiteralArguments(ArrayInit init) or
74+
TListArguments(CommandArgumentList list) or
75+
TMutableArguments(CommandArgArrayImmutableFirst array)
76+
77+
/** An ordered sequence of command arguments. */
78+
private class CommandArguments extends TCommandArguments {
79+
string toString() { result = this.getArgument(0).toString() }
80+
81+
/** Gets a possible value at the given index. */
82+
Expr getArgument(int index) {
83+
exists(ArrayInit init |
84+
this = TLiteralArguments(init) and
85+
isInlineArgumentArray(init) and
86+
result = init.getInit(index)
87+
)
88+
or
89+
exists(CommandArgumentList list |
90+
this = TListArguments(list) and
91+
result = list.getArgument(index)
92+
)
93+
or
94+
exists(CommandArgArrayImmutableFirst array |
95+
this = TMutableArguments(array) and
96+
result = array.getAnElement(index)
97+
)
98+
}
99+
100+
/** Gets the single string literal stored at the given index, if there is one. */
101+
private StringLiteral getFixedLiteralArgument(int index) {
102+
result = this.getArgument(index) and
103+
forall(Expr value | value = this.getArgument(index) | value = result)
104+
}
105+
106+
predicate hasSafeArgument(int index) {
107+
exists(this.getArgument(index)) and
108+
not isShell(this.getArgument(0)) and
109+
(
110+
not exists(delegatedCommandIndex(this.getArgument(0)))
111+
or
112+
exists(int commandIndex |
113+
commandIndex = delegatedCommandIndex(this.getArgument(0)) and
114+
(
115+
index < commandIndex
116+
or
117+
index > commandIndex and
118+
exists(StringLiteral command |
119+
command = this.getFixedLiteralArgument(commandIndex) and
120+
not isShell(command) and
121+
not exists(delegatedCommandIndex(command))
122+
)
123+
)
124+
)
125+
)
126+
}
53127
}
54128

55129
/**
@@ -101,23 +175,25 @@ private class CommandArgumentList extends SsaExplicitWrite {
101175
result.getQualifier() = this.getAUseBeforeFirstAdd()
102176
}
103177

104-
/** Gets an addition to this list which is not the first element. */
105-
MethodCall getASubsequentAdd() {
106-
result = this.getAnAdd() and
107-
not result = this.getAFirstAdd()
108-
}
109-
110-
/** Holds if the first element of this list is a shell command. */
111-
predicate isShell() {
112-
exists(MethodCall ma | ma = this.getAFirstAdd() and isShell(ma.getArgument(0)))
178+
/** Gets a single-element addition at the given index. */
179+
private MethodCall getAnAddAtIndex(int index) {
180+
index = 0 and
181+
result = this.getAFirstAdd() and
182+
result.getMethod().hasName("add") and
183+
result.getNumArgument() = 1
184+
or
185+
index > 0 and
186+
exists(MethodCall previous |
187+
previous = this.getAnAddAtIndex(index - 1) and
188+
result = this.getAnAdd() and
189+
result.getMethod().hasName("add") and
190+
result.getNumArgument() = 1 and
191+
adjacentUseUse(previous.getQualifier(), result.getQualifier())
192+
)
113193
}
114-
}
115194

116-
/**
117-
* The type `String[]`.
118-
*/
119-
private class ArrayOfStringType extends Array {
120-
ArrayOfStringType() { this.getElementType() instanceof TypeString }
195+
/** Gets the argument added at the given index. */
196+
Expr getArgument(int index) { result = this.getAnAddAtIndex(index).getArgument(0) }
121197
}
122198

123199
private predicate arrayVarWrite(ArrayAccess acc) { exists(Assignment a | a.getDest() = acc) }
@@ -153,7 +229,7 @@ private class CommandArgumentArray extends SsaExplicitWrite {
153229
*/
154230
private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
155231
CommandArgArrayImmutableFirst() {
156-
(exists(this.getAWrite(0)) or exists(firstElementOf(this.getDefiningExpr()))) and
232+
(exists(this.getAWrite(0)) or exists(arrayElementOf(this.getDefiningExpr(), 0))) and
157233
forall(VarRead use | exists(this.getAWrite(0, use)) | use = ssaGetAFirstUse(this))
158234
}
159235

@@ -162,41 +238,50 @@ private class CommandArgArrayImmutableFirst extends CommandArgumentArray {
162238
result = this.getAWrite(0)
163239
or
164240
not exists(this.getAWrite(0)) and
165-
result = firstElementOf(this.getDefiningExpr())
241+
result = arrayElementOf(this.getDefiningExpr(), 0)
166242
}
167243

168-
/** Holds if the first element of this array is a shell command. */
169-
predicate isShell() { isShell(this.getFirstElement()) }
244+
/** Gets a possible value for the element at the given index. */
245+
Expr getAnElement(int index) {
246+
index = 0 and result = this.getFirstElement()
247+
or
248+
index > 0 and
249+
(
250+
result = this.getAWrite(index)
251+
or
252+
result = arrayElementOf(this.getDefiningExpr(), index)
253+
)
254+
}
170255
}
171256

172-
/** Gets the first element of an imutable array of strings */
173-
private Expr firstElementOf(Expr arr) {
257+
/** Gets an element of an immutable array of strings. */
258+
private Expr arrayElementOf(Expr arr, int index) {
174259
arr.getType() instanceof ArrayOfStringType and
175260
(
176-
result = firstElementOf(arr.(Assignment).getRhs())
261+
result = arrayElementOf(arr.(Assignment).getRhs(), index)
177262
or
178-
result = firstElementOf(arr.(LocalVariableDeclExpr).getInit())
263+
result = arrayElementOf(arr.(LocalVariableDeclExpr).getInit(), index)
179264
or
180265
exists(CommandArgArrayImmutableFirst caa | arr = caa.getARead() |
181-
result = caa.getFirstElement()
266+
result = caa.getAnElement(index)
182267
)
183268
or
184269
exists(MethodCall ma, Method m |
185270
arr = ma and
186271
ma.getMethod() = m and
187272
m.getDeclaringType().hasQualifiedName("java.util", "Arrays") and
188273
m.hasName("copyOf") and
189-
result = firstElementOf(ma.getArgument(0))
274+
result = arrayElementOf(ma.getArgument(0), index)
190275
)
191276
or
192277
exists(Field f |
193278
f.isStatic() and
194279
arr.(FieldRead).getField() = f and
195-
result = firstElementOf(f.getInitializer())
280+
result = arrayElementOf(f.getInitializer(), index)
196281
)
197282
or
198-
result = arr.(ArrayInit).getInit(0)
283+
result = arr.(ArrayInit).getInit(index)
199284
or
200-
result = arr.(ArrayCreationExpr).getInit().getInit(0)
285+
result = arr.(ArrayCreationExpr).getInit().getInit(index)
201286
)
202287
}

0 commit comments

Comments
 (0)