mirror of
https://github.com/github/codeql.git
synced 2026-04-24 00:05:14 +02:00
Apply suggestions from code review
Co-authored-by: Ben Ahmady <32935794+subatoi@users.noreply.github.com>
This commit is contained in:
@@ -10,7 +10,7 @@ allows the user to execute malicious code.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>If possible, use APIs that don't run shell commands and accept command
|
||||
<p>If possible, use APIs that don't run shell commands and that accept command
|
||||
arguments as an array of strings rather than a single concatenated string. This
|
||||
is both safer and more portable.</p>
|
||||
|
||||
@@ -26,13 +26,13 @@ string is safe before using it.</p>
|
||||
<example>
|
||||
<p>The following example shows code that extracts a filename from an HTTP query
|
||||
parameter that may contain untrusted data, and then embeds it into a shell
|
||||
command to count its lines without examining it first.</p>
|
||||
command to count its lines without examining it first:</p>
|
||||
|
||||
<sample src="examples/command-injection.js" />
|
||||
|
||||
<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>
|
||||
<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>
|
||||
|
||||
<p>To avoid this catastrophic loophole, use an API such as
|
||||
<p>To avoid this catastrophic behavior, use an API such as
|
||||
<code>child_process.execFileSync</code> that does not spawn a shell by
|
||||
default:</p>
|
||||
|
||||
|
||||
@@ -91,8 +91,8 @@ into an array of arguments instead.
|
||||
|
||||
<p>
|
||||
|
||||
If we want to allow the user to specify other options to
|
||||
<code>node</code>, we can use a library like
|
||||
If you want to allow the user to specify other options to
|
||||
<code>node</code>, you can use a library like
|
||||
<code>shell-quote</code> to parse the user input into an array of
|
||||
arguments without risking command injection:
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@
|
||||
<p>
|
||||
|
||||
As another example, consider the following code which is similar to the
|
||||
above, but pipes the output of <code>wget</code> into <code>wc -l</code>
|
||||
preceding example, but pipes the output of <code>wget</code> into <code>wc -l</code>
|
||||
to count the number of lines in the downloaded file.
|
||||
|
||||
</p>
|
||||
@@ -89,7 +89,7 @@
|
||||
<p>
|
||||
|
||||
In this case, using <code>child_process.execFile</code> is not an option
|
||||
because the shell is needed to interpret the pipe operator. Instead, we
|
||||
because the shell is needed to interpret the pipe operator. Instead, you
|
||||
can use <code>shell-quote</code> to escape the input before embedding it
|
||||
in the command:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user