IncompleteSanitization.qhelp

<!DOCTYPE qhelp PUBLIC
  "-//Semmle//qhelp//EN"
  "qhelp.dtd">
<qhelp>

<overview>
<p>
Sanitizing untrusted input is a common technique for preventing injection attacks such as
SQL injection or cross-site scripting. Usually, this is done by escaping meta-characters such
as quotes in a domain-specific way so that they are treated as normal characters.
</p>
<p>
However, directly using the string <code>replace</code> method to perform escaping is notoriously
error-prone. Common mistakes include only replacing the first occurrence
of a meta-character, or backslash-escaping various meta-characters but not the backslash itself.
</p>
<p>
In the former case, later meta-characters are left undisturbed and can be used to subvert the
sanitization. In the latter case, preceding a meta-character with a backslash leads to the
backslash being escaped, but the meta-character appearing un-escaped, which again makes the
sanitization ineffective.
</p>
<p>
Even if the escaped string is not used in a security-critical context, incomplete escaping may
still have undesirable effects, such as badly rendered or confusing output.
</p>
</overview>

<recommendation>
<p>
Use a (well-tested) sanitization library if at all possible. These libraries are much more
likely to handle corner cases correctly than a custom implementation.
</p>

<p>

An even safer alternative is to design the application so that sanitization is not
needed, for instance by using prepared statements for SQL queries.

</p>

<p>
Otherwise, make sure to use a regular expression with the <code>g</code> flag to ensure that
all occurrences are replaced, and remember to escape backslashes if applicable.
</p>
</recommendation>

<example>
<p>
For example, assume that we want to embed a user-controlled string <code>accountNumber</code>
into a SQL query as part of a string literal. To avoid SQL injection, we need to ensure that
the string does not contain un-escaped single-quote characters. The following function attempts
to ensure this by doubling single quotes, and thereby escaping them:
</p>

<sample src="examples/IncompleteSanitization.js" />

<p>
As written, this sanitizer is ineffective: if the first argument to <code>replace</code> is
a string literal (as in this case), only the <i>first</i> occurrence of that string is
replaced.
</p>

<p>
As mentioned above, the function <code>escapeQuotes</code> should be replaced with a purpose-built
sanitization library, such as the npm module <code>sqlstring</code>. Many other sanitization
libraries are available from npm and other sources.
</p>

<p>
If this is not an option, <code>escapeQuotes</code> should be rewritten to use a regular expression
with the <code>g</code> ("global") flag instead:
</p>

<sample src="examples/IncompleteSanitizationGood.js" />

<p>
Note that it is very important to include the global flag: <code>s.replace(/'/, "''")</code>
<i>without</i> the global flag is equivalent to the first example above and only replaces the
first quote.
</p>
</example>

<references>
<li>OWASP Top 10: <a href="https://www.owasp.org/index.php/Top_10-2017_A1-Injection">A1 Injection</a>.</li>
<li>npm: <a href="https://www.npmjs.com/package/sqlstring">sqlstring</a> package.</li>
</references>
</qhelp>