Skip to content

Commit 921d8de

Browse files
Apply suggestions from code review
Co-authored-by: Erik Krogh Kristensen <[email protected]>
1 parent 5fb6b58 commit 921d8de

File tree

1 file changed

+5
-6
lines changed

1 file changed

+5
-6
lines changed

javascript/ql/src/Security/CWE-078/CommandInjection.inc.qhelp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
<overview>
77
<p>Code that passes untrusted user input directly to
8-
<code>child_process.exec</code> or similar APIs that execute commands by
9-
spawning a shell allows the user to execute malicious code.</p>
8+
<code>child_process.exec</code> or similar APIs that execute shell commands
9+
allows the user to execute malicious code.</p>
1010
</overview>
1111

1212
<recommendation>
@@ -17,9 +17,8 @@ user input and then choose among hard-coded string literals.</p>
1717
<p>If the applicable libraries or commands cannot be determined until runtime,
1818
then add code to verify that the user input string is safe before using it.</p>
1919

20-
<p>If possible, prefer APIs that run the commands directly rather than via a
21-
shell, and that accept command arguments as an array of strings rather than a
22-
single concatenated string. This is both safer and more portable.</p>
20+
<p>If possible, use APIs that don't run shell commands, and accept command arguments
21+
as an array of strings rather than a single concatenated string. This is both safer and more portable.</p>
2322

2423
<p>In the latter case, if you are given the arguments as a single string, note
2524
that it is not safe to simply split the string on whitespace, since an argument
@@ -41,7 +40,7 @@ passing a filename like <code>foo.txt; rm -rf .</code>, which will first count
4140
the lines in <code>foo.txt</code> and then delete all files in the current
4241
directory.</p>
4342

44-
<p>To avoid this potentially catastrophic loophole, use an API like
43+
<p>To avoid this catastrophic loophole, use an API like
4544
<code>child_process.execFileSync</code> that does not spawn a shell by
4645
default:</p>
4746

0 commit comments

Comments
 (0)