Skip to content

Commit ae23724

Browse files
Apply suggestions from code review
Co-authored-by: Ben Ahmady <[email protected]>
1 parent 63c45a0 commit ae23724

File tree

3 files changed

+8
-8
lines changed

3 files changed

+8
-8
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ allows the user to execute malicious code.</p>
1010
</overview>
1111

1212
<recommendation>
13-
<p>If possible, use APIs that don't run shell commands and accept command
13+
<p>If possible, use APIs that don't run shell commands and that accept command
1414
arguments as an array of strings rather than a single concatenated string. This
1515
is both safer and more portable.</p>
1616

@@ -26,13 +26,13 @@ string is safe before using it.</p>
2626
<example>
2727
<p>The following example shows code that extracts a filename from an HTTP query
2828
parameter that may contain untrusted data, and then embeds it into a shell
29-
command to count its lines without examining it first.</p>
29+
command to count its lines without examining it first:</p>
3030

3131
<sample src="examples/command-injection.js" />
3232

33-
<p>A malicious user can take advantage of this code by executing arbitrary shell commands. For instance, by providing a filename like <code>foo.txt; rm -rf .</code>, the user can first count the lines in <code>foo.txt</code> and subsequently delete all files in the current directory. </p>
33+
<p>A malicious user can take advantage of this code by executing arbitrary shell commands. For example, by providing a filename like <code>foo.txt; rm -rf .</code>, the user can first count the lines in <code>foo.txt</code> and subsequently delete all files in the current directory. </p>
3434

35-
<p>To avoid this catastrophic loophole, use an API such as
35+
<p>To avoid this catastrophic behavior, use an API such as
3636
<code>child_process.execFileSync</code> that does not spawn a shell by
3737
default:</p>
3838

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ into an array of arguments instead.
9191

9292
<p>
9393

94-
If we want to allow the user to specify other options to
95-
<code>node</code>, we can use a library like
94+
If you want to allow the user to specify other options to
95+
<code>node</code>, you can use a library like
9696
<code>shell-quote</code> to parse the user input into an array of
9797
arguments without risking command injection:
9898

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@
7979
<p>
8080

8181
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>
82+
preceding example, but pipes the output of <code>wget</code> into <code>wc -l</code>
8383
to count the number of lines in the downloaded file.
8484

8585
</p>
@@ -89,7 +89,7 @@
8989
<p>
9090

9191
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
92+
because the shell is needed to interpret the pipe operator. Instead, you
9393
can use <code>shell-quote</code> to escape the input before embedding it
9494
in the command:
9595

0 commit comments

Comments
 (0)