Skip to content

Commit 59001bb

Browse files
committed
add qhelp for js/shell-command-constructed-from-input
1 parent 5e647da commit 59001bb

File tree

3 files changed

+85
-0
lines changed

3 files changed

+85
-0
lines changed
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
8+
Dynamically constructing a shell command with inputs from exported
9+
functions, may inadvertently change the meaning of the shell command.
10+
11+
Clients using the exported function may use inputs that contains
12+
characters that the shell interprets in a special way, for instance
13+
quotes and spaces.
14+
15+
This can result in the shell command misbehaving, or even
16+
allowing a malicious user to execute arbitrary commands on the system.
17+
</p>
18+
19+
20+
</overview>
21+
<recommendation>
22+
23+
<p>
24+
If possible, use hard-coded string literals to specify the
25+
shell command to run, and provide the dynamic arguments to the shell
26+
command separately to avoid interpretation by the shell.
27+
</p>
28+
29+
<p>
30+
Alternatively, if the shell command must be constructed
31+
dynamically, then add code to ensure that special characters
32+
do not alter the shell command unexpectedly.
33+
</p>
34+
35+
</recommendation>
36+
<example>
37+
38+
<p>
39+
The following example shows a dynamically constructed shell
40+
command that downloads a file from a remote url.
41+
</p>
42+
43+
<sample src="examples/unsafe-shell-command-construction.js" />
44+
45+
<p>
46+
The shell command will, however, fail to work as intended if the
47+
input contains spaces or other special characters interpreted in a
48+
special way by the shell.
49+
</p>
50+
51+
<p>
52+
Even worse, although less likely, a malicious user could
53+
provide the input <code>http://example.org; cat /etc/passwd</code>
54+
in order to execute the command <code>cat /etc/passwd</code>.
55+
56+
</p>
57+
58+
<p>
59+
To avoid such potentially catastrophic behaviors, provide the
60+
inputs from exported functions as an argument that does not
61+
get interpreted by a shell:
62+
</p>
63+
64+
<sample src="examples/unsafe-shell-command-construction_fixed.js" />
65+
66+
</example>
67+
<references>
68+
69+
<li>
70+
OWASP:
71+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
72+
</li>
73+
74+
</references>
75+
</qhelp>
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var cp = require("child_process");
2+
3+
module.exports = function download(path, callback) {
4+
cp.exec("wget " + path, callback);
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
var cp = require("child_process");
2+
3+
module.exports = function download(path, callback) {
4+
cp.execFile("wget", [path], callback);
5+
}

0 commit comments

Comments
 (0)