Skip to content

Commit 5e647da

Browse files
erik-krogherik-krogh
committed
add js/shell-command-constructed-from-input query
1 parent a1a6826 commit 5e647da

File tree

3 files changed

+297
-0
lines changed

3 files changed

+297
-0
lines changed

‎javascript/ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Unsafe shell command constructed from library input
3+
* @description Using externally controlled strings in a command line may allow a malicious
4+
* user to change the meaning of the command.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id js/shell-command-constructed-from-input
9+
* @tags correctness
10+
* security
11+
* external/cwe/cwe-078
12+
* external/cwe/cwe-088
13+
*/
14+
15+
import javascript
16+
import semmle.javascript.security.dataflow.UnsafeShellCommandConstruction::UnsafeShellCommandConstruction
17+
import DataFlow::PathGraph
18+
19+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
20+
where cfg.hasFlowPath(source, sink) and sinkNode = sink.getNode()
21+
select sinkNode.getHighLight(), source, sink, "$@ based on libary input is later used in $@.",
22+
sinkNode.getHighLight(), sinkNode.getSinkType(), sinkNode.getCommandExecution(), "shell command"

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstruction.qll‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Provides a taint tracking configuration for reasoning about shell command
3+
* constructed from library input vulnerabilities (CWE-078).
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `UnsafeShellCommandConstruction::Configuration` is needed, otherwise
7+
* `UnsafeShellCommandConstructionCustomizations` should be imported instead.
8+
*/
9+
10+
import javascript
11+
12+
/**
13+
* Classes and predicates for the shell command constructed from library input query.
14+
*/
15+
module UnsafeShellCommandConstruction {
16+
import UnsafeShellCommandConstructionCustomizations::UnsafeShellCommandConstruction
17+
18+
/**
19+
* A taint-tracking configuration for reasoning about shell command constructed from library input vulnerabilities.
20+
*/
21+
class Configuration extends TaintTracking::Configuration {
22+
Configuration() { this = "UnsafeLibaryCommandInjection" }
23+
24+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
25+
26+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
27+
28+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
29+
30+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
31+
guard instanceof PathExistsSanitizerGuard or
32+
guard instanceof TaintTracking::AdHocWhitelistCheckSanitizer
33+
}
34+
}
35+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* shell command constructed from library input vulnerabilities,
4+
* as well as extension points for adding your own.
5+
*/
6+
7+
import javascript
8+
import semmle.javascript.security.dataflow.RemoteFlowSources
9+
10+
/**
11+
* Module containing sources, sinks, and sanitizers for shell command constructed from library input.
12+
*/
13+
module UnsafeShellCommandConstruction {
14+
import IndirectCommandArgument
15+
16+
/**
17+
* A data flow source for shell command constructed from library input.
18+
*/
19+
abstract class Source extends DataFlow::Node { }
20+
21+
/**
22+
* A data flow sink for shell command constructed from library input.
23+
*/
24+
abstract class Sink extends DataFlow::Node {
25+
/**
26+
* Gets a description how the shell command is constructed for this sink.
27+
*/
28+
abstract string getSinkType();
29+
30+
/**
31+
* Gets the dataflow node that executes the shell command.
32+
*/
33+
abstract SystemCommandExecution getCommandExecution();
34+
35+
/**
36+
* Gets the node that should be highlighted for this sink.
37+
* E.g. for a string concatenation, the sink is one of the leafs and the highlight is the concatenation root.
38+
*/
39+
abstract DataFlow::Node getHighLight();
40+
}
41+
42+
/**
43+
* A sanitizer for shell command constructed from library input.
44+
*/
45+
abstract class Sanitizer extends DataFlow::Node { }
46+
47+
/**
48+
* Gets the number of occurrences of "/" in `path`.
49+
*/
50+
bindingset[path]
51+
private int countSlashes(string path) {
52+
not exists(path.indexOf("/")) and result = 0
53+
or
54+
result = max(int n | exists(path.indexOf("/", n, 0)) | n)
55+
}
56+
57+
/**
58+
* Gets the topmost package.json that appears in the project.
59+
*
60+
* There can be multiple results if the there exists multiple package.json that are equally deeply nested in the folder structure.
61+
* Results are limited to package.json files that are at most nested 2 directories deep.
62+
*/
63+
private PackageJSON getTopmostPackageJSON() {
64+
result =
65+
min(PackageJSON j |
66+
countSlashes(j.getFile().getRelativePath()) <= 2
67+
|
68+
j order by countSlashes(j.getFile().getRelativePath())
69+
)
70+
}
71+
72+
/**
73+
* Gets a value exported by the main module from a package.json.
74+
* The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
75+
*/
76+
private DataFlow::Node getAnExportedValue() {
77+
exists(PackageJSON pack | pack = getTopmostPackageJSON() |
78+
result = getAnExportFromModule(pack.getMainModule())
79+
)
80+
or
81+
result = getAnExportedValue().(DataFlow::PropWrite).getRhs()
82+
or
83+
exists(DataFlow::SourceNode callee |
84+
callee = getAnExportedValue().(DataFlow::NewNode).getCalleeNode().getALocalSource()
85+
|
86+
result = callee.getAPropertyRead("prototype").getAPropertyWrite()
87+
or
88+
result = callee.(DataFlow::ClassNode).getAnInstanceMethod()
89+
)
90+
or
91+
result = getAnExportedValue().getALocalSource()
92+
or
93+
result = getAnExportedValue().(DataFlow::SourceNode).getAPropertyReference()
94+
or
95+
exists(Module mod | mod = getAnExportedValue().getEnclosingExpr().(Import).getImportedModule() |
96+
result = getAnExportFromModule(mod)
97+
)
98+
or
99+
exists(DataFlow::ClassNode cla | cla = getAnExportedValue() |
100+
result = cla.getAnInstanceMethod() or
101+
result = cla.getAStaticMethod() or
102+
result = cla.getConstructor()
103+
)
104+
}
105+
106+
/**
107+
* Gets an exported node from the module `mod`.
108+
*/
109+
private DataFlow::Node getAnExportFromModule(Module mod) {
110+
result.analyze().getAValue() = mod.(NodeModule).getAModuleExportsValue()
111+
or
112+
exists(ASTNode export | result.getEnclosingExpr() = export | mod.exports(_, export))
113+
}
114+
115+
/**
116+
* A parameter of an exported function, seen as a source for shell command constructed from library input.
117+
*/
118+
class ExternalInputSource extends Source, DataFlow::ParameterNode {
119+
ExternalInputSource() {
120+
this = getAnExportedValue().(DataFlow::FunctionNode).getAParameter() and
121+
not this.getName() = ["cmd", "command"] // looks to be on purpose.
122+
}
123+
}
124+
125+
/**
126+
* Gets a node that is later executed as a shell command in the command execution `sys`.
127+
*/
128+
private DataFlow::Node isExecutedAsShellCommand(
129+
DataFlow::TypeBackTracker t, SystemCommandExecution sys
130+
) {
131+
t.start() and result = sys.getACommandArgument() and sys.isShellInterpreted(result)
132+
or
133+
t.start() and isIndirectCommandArgument(result, sys)
134+
or
135+
exists(DataFlow::TypeBackTracker t2 |
136+
t2 = t.smallstep(result, isExecutedAsShellCommand(t2, sys))
137+
)
138+
}
139+
140+
/**
141+
* A string concatenation that is later executed as a shell command.
142+
*/
143+
class StringConcatEndingInCommandExecutionSink extends Sink, StringOps::ConcatenationLeaf {
144+
SystemCommandExecution sys;
145+
StringOps::ConcatenationRoot root;
146+
147+
StringConcatEndingInCommandExecutionSink() {
148+
this = root.getALeaf() and
149+
root = isExecutedAsShellCommand(DataFlow::TypeBackTracker::end(), sys) and
150+
exists(string prev | prev = this.getPreviousLeaf().getStringValue() |
151+
prev.regexpMatch(".* ('|\")?[0-9a-zA-Z/]*")
152+
)
153+
}
154+
155+
override string getSinkType() { result = "String concatenation" }
156+
157+
override SystemCommandExecution getCommandExecution() { result = sys }
158+
159+
override DataFlow::Node getHighLight() { result = root }
160+
}
161+
162+
/**
163+
* An element pushed to an array, where the array is later used to execute a shell command.
164+
*/
165+
class ArrayAppendEndingInCommandExecutinSink extends Sink {
166+
DataFlow::SourceNode array;
167+
SystemCommandExecution sys;
168+
169+
ArrayAppendEndingInCommandExecutinSink() {
170+
this =
171+
[array.(DataFlow::ArrayCreationNode).getAnElement(),
172+
array.getAMethodCall(["push", "unshift"]).getAnArgument()] and
173+
exists(DataFlow::MethodCallNode joinCall | array.getAMethodCall("join") = joinCall |
174+
joinCall = isExecutedAsShellCommand(DataFlow::TypeBackTracker::end(), sys) and
175+
joinCall.getNumArgument() = 1 and
176+
joinCall.getArgument(0).getStringValue() = " "
177+
)
178+
}
179+
180+
override string getSinkType() { result = "Array element" }
181+
182+
override SystemCommandExecution getCommandExecution() { result = sys }
183+
184+
override DataFlow::Node getHighLight() { result = this }
185+
}
186+
187+
/**
188+
* A formatted string that is later executed as a shell command.
189+
*/
190+
class FormatedStringInCommandExecutionSink extends Sink {
191+
PrintfStyleCall call;
192+
SystemCommandExecution sys;
193+
194+
FormatedStringInCommandExecutionSink() {
195+
this = call.getFormatArgument(_) and
196+
call = isExecutedAsShellCommand(DataFlow::TypeBackTracker::end(), sys) and
197+
exists(string formatString | call.getFormatString().mayHaveStringValue(formatString) |
198+
formatString.regexpMatch(".* ('|\")?[0-9a-zA-Z/]*%.*")
199+
)
200+
}
201+
202+
override string getSinkType() { result = "Formatted string" }
203+
204+
override SystemCommandExecution getCommandExecution() { result = sys }
205+
206+
override DataFlow::Node getHighLight() { result = this }
207+
}
208+
209+
/**
210+
* A sanitizer like: "'"+name.replace(/'/g,"'\\''")+"'"
211+
* Which sanitizes on Unix.
212+
* The sanitizer is only safe if sorounded by single-quotes, which is assumed.
213+
*/
214+
class ReplaceQuotesSanitizer extends Sanitizer, StringReplaceCall {
215+
ReplaceQuotesSanitizer() {
216+
this.getAReplacedString() = "'" and
217+
this.isGlobal() and
218+
this.getRawReplacement().mayHaveStringValue(["'\\''", ""])
219+
}
220+
}
221+
222+
/**
223+
* A sanitizer that sanitizers paths that exist in the file-system.
224+
* For example: `x` is sanitized in `fs.existsSync(x)` or `fs.existsSync(x + "/suffix/path")`.
225+
*/
226+
class PathExistsSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
227+
PathExistsSanitizerGuard() {
228+
this = DataFlow::moduleMember("path", "exist").getACall() or
229+
this = DataFlow::moduleMember("fs", "existsSync").getACall()
230+
}
231+
232+
override predicate sanitizes(boolean outcome, Expr e) {
233+
outcome = true and
234+
(
235+
e = getArgument(0).asExpr() or
236+
e = getArgument(0).(StringOps::ConcatenationRoot).getALeaf().asExpr()
237+
)
238+
}
239+
}
240+
}

0 commit comments

Comments
 (0)