Skip to content

Commit 63c45a0

Browse files
author
Max Schaefer
committed
Add another example of when and how to use shell-quote.
1 parent 1d3e344 commit 63c45a0

File tree

4 files changed

+36
-19
lines changed

4 files changed

+36
-19
lines changed

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

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,21 @@
2828

2929
<p>
3030

31-
If given arguments as a single string, avoid simply splitting the string on
32-
whitespace. Arguments may contain quoted whitespace, causing them to split into
33-
multiple arguments. Use a library like <code>shell-quote</code> to parse the string
34-
into an array of arguments instead.
31+
If given arguments as a single string, avoid simply splitting the string
32+
on whitespace. Arguments may contain quoted whitespace, causing them to
33+
split into multiple arguments. Use a library like
34+
<code>shell-quote</code> to parse the string into an array of arguments
35+
instead.
3536

3637
</p>
3738

3839
<p>
39-
Alternatively, if the shell command must be constructed
40-
dynamically, then add code to ensure that special characters
41-
do not alter the shell command unexpectedly.
40+
41+
Alternatively, if the command must be interpreted by a shell (for
42+
example because it includes I/O redirections), you can use
43+
<code>shell-quote</code> to escape any special characters in the input
44+
before embedding it in the command.
45+
4246
</p>
4347

4448
</recommendation>
@@ -74,15 +78,24 @@ into an array of arguments instead.
7478

7579
<p>
7680

77-
If you want to allow the user to specify other options to
78-
<code>wget</code> as a string, we can use a library like
79-
<code>shell-quote</code>
80-
to parse the user input into an array of arguments without risking
81-
command injection:
81+
As another example, consider the following code which is similar to the
82+
above, but pipes the output of <code>wget</code> into <code>wc -l</code>
83+
to count the number of lines in the downloaded file.
84+
85+
</p>
86+
87+
<sample src="examples/unsafe-shell-command-construction_pipe.js" />
88+
89+
<p>
90+
91+
In this case, using <code>child_process.execFile</code> is not an option
92+
because the shell is needed to interpret the pipe operator. Instead, we
93+
can use <code>shell-quote</code> to escape the input before embedding it
94+
in the command:
8295

8396
</p>
8497

85-
<sample src="examples/unsafe-shell-command-construction_shellquote.js" />
98+
<sample src="examples/unsafe-shell-command-construction_pipe_fixed.js" />
8699

87100
</example>
88101
<references>
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 + " | wc -l", 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.exec("wget " + shellQuote.quote([path]) + " | wc -l", callback);
5+
};

javascript/ql/src/Security/CWE-078/examples/unsafe-shell-command-construction_shellquote.js

Lines changed: 0 additions & 6 deletions
This file was deleted.

0 commit comments

Comments
 (0)