10000 JS: add heuristic variants of queries that use RemoteFlowSource · github/codeql@a727142 · GitHub
[go: up one dir, main page]

Skip to content

Commit a727142

Browse files
committed
JS: add heuristic variants of queries that use RemoteFlowSource
1 parent 02c199e commit a727142

34 files changed

+1518
-0
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Using unsanitized untrusted data in an external API can cause a variety of security issues. This query reports
7+
external APIs that use untrusted data. The results are not filtered so that you can audit all examples. The query provides data for security reviews of the application and you can also use it to identify external APIs that should be modeled as either taint steps, or sinks for specific problems.</p>
8+
9+
<p>An external API is defined as a method call to a method that is not defined in the source code, not overridden
10+
in the source code, and is not modeled as a taint step in the default taint library. External APIs may be from the
11+
third-party dependencies or from internal dependencies. The query reports uses of
12+
untrusted data one of the arguments of external API call or in the return value from a callback passed to an external API.</p>
13+
14+
</overview>
15+
<recommendation>
16+
17+
<p>For each result:</p>
18+
19+
<ul>
20+
<li>If the result highlights a known sink, confirm that the result is reported by the relevant query, or
21+
that the result is a false positive because this data is sanitized.</li>
22+
<li>If the result highlights an unknown sink for a problem, then add modeling for the sink to the relevant query,
23+
and confirm that the result is either found, or is safe due to appropriate sanitization.</li>
24+
<li>If the result represents a call to an external API that transfers taint, add the appropriate modeling, and
25+
re-run the query to determine what new results have appeared due to this additional modeling.</li>
26+
</ul>
27+
28+
<p>Otherwise, the result is likely uninteresting. Custom versions of this query can extend the <code>SafeExternalAPIMethod</code>
29+
class to exclude known safe external APIs from future analysis.</p>
30+
31+
</recommendation>
32+
<example>
33+
34+
<p>In this first example, a query parameter is read from the <code>req</code> parameter and then ultimately used in a call to the
35+
<code>res.send</code> external API:</p>
36+
37+
<sample src="ExternalAPISinkExample.js" />
38+
39+
<p>This is a reflected XSS sink. The XSS query should therefore be reviewed to confirm that this sink is appropriately modeled,
40+
and if it is, to confirm that the query reports this particular result, or that the result is a false positive due to
41+
some existing sanitization.</p>
42+
43+
<p>In this second example, again a query parameter is read from <code>req</code>.</p>
44+
45+
<sample src="ExternalAPITaintStepExample.js" />
46+
47+
<p>If the query reported the call to <code>path.join</code> on line 4, this would suggest that this external API is
48+
not currently modeled as a taint step in the taint tracking library. The next step would be to model this as a taint step, then
49+
re-run the query to determine what additional results might be found. In this example, it seems the result of the
50+
<code>path.join</code> will be used as a file path, leading to a path traversal vulnerability.</p>
51+
52+
<p>Note that both examples are correctly handled by the standard taint tracking library and security queries.</p>
53+
</example>
54+
<references>
55+
56+
</references>
57+
</qhelp>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Untrusted data passed to external API
3+
* @description Data provided remotely is used in this external API without sanitization, which could be a security risk.
4+
* @id js/untrusted-data-to-external-api
5+
* @kind path-problem
6+
* @precision low
7+
* @problem.severity error
8+
* @security-severity 7.8
9+
* @tags security external/cwe/cwe-20
10+
*/
11+
12+
import javascript
13+
import semmle.javascript.security.dataflow.ExternalAPIUsedWithUntrustedDataQuery
14+
import DataFlow::PathGraph
15+
import semmle.javascript.heuristics.AdditionalSources
16+
17+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
18+
where config.hasFlowPath(source, sink)
19+
select sink, source, sink,
20+
"Call to " + sink.getNode().(Sink).getApiName() + " with untrusted data from $@.", source,
21+
source.toString()
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Code that passes user input directly to
7+
<code>require('child_process').exec</code>, or some other library
8+
routine that executes a command, allows the user to execute malicious
9+
code.</p>
10+
11+
</overview>
12+
<recommendation>
13+
14+
<p>If possible, use hard-coded string literals to specify the command to run
15+
or library to load. Instead of passing the user input directly to the
16+
process or library function, examine the user input and then choose
17+
among hard-coded string literals.</p>
18+
19+
<p>If the applicable libraries or commands cannot be determined at
20+
compile time, then add code to verify that the user input string is
21+
safe before using it.</p>
22+
23+
</recommendation>
24+
<example>
25+
26+
<p>The following example shows code that takes a shell script that can be changed
27+
maliciously by a user, and passes it straight to <code>child_process.exec</code>
28+
without examining it first.</p>
29+
30+
<sample src="examples/command-injection.js" />
31+
32+
</example>
33+
<references>
34+
35+
<li>
36+
OWASP:
37+
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
38+
</li>
39+
40+
<!-- LocalWords: CWE untrusted unsanitized Runtime
41+
-->
42+
43+
</references>
44+
</qhelp>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @name Uncontrolled command line
3+
* @description Using externally controlled strings in a command line may allow a malicious
4+
* user to change the meaning of the command.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.8
8+
* @precision high
9+
* @id js/command-line-injection
10+
* @tags correctness
11+
* security
12+
* external/cwe/cwe-078
13+
* external/cwe/cwe-088
14+
*/
15+
16+
import javascript
17+
import semmle.javascript.security.dataflow.CommandInjectionQuery
18+
import DataFlow::PathGraph
19+
import semmle.javascript.heuristics.AdditionalSources
20+
21+
from
22+
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node highlight,
23+
Source sourceNode
24+
where
25+
cfg.hasFlowPath(source, sink) and
26+
(
27+
if cfg.isSinkWithHighlight(sink.getNode(), _)
28+
then cfg.isSinkWithHighlight(sink.getNode(), highlight)
29+
else highlight = sink.getNode()
30+
) and
31+
sourceNode = source.getNode()
32+
select highlight, source, sink, "This command line depends on a $@.", source.getNode(),
33+
"user-provided value"
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Directly writing user input (for example, a URL query parameter) to a webpage
9+
without properly sanitizing the input first, allows for a cross-site scripting vulnerability.
10+
</p>
11+
<p>
12+
This kind of vulnerability is also called <i>DOM-based</i> cross-site scripting, to distinguish
13+
it from other types of cross-site scripting.
14+
</p>
15+
</overview>
16+
17+
<recommendation>
18+
<p>
19+
To guard against cross-site scripting, consider using contextual output encoding/escaping before
20+
writing user input to the page, or one of the other solutions that are mentioned in the
21+
references.
22+
</p>
23+
</recommendation>
24+
25+
<example>
26+
<p>
27+
The following example shows part of the page URL being written directly to the document,
28+
leaving the website vulnerable to cross-site scripting.
29+
</p>
30+
<sample src="examples/Xss.js" />
31+
</example>
32+
33+
<references>
34+
<li>
35+
OWASP:
36+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/DOM_based_XSS_Prevention_Cheat_Sheet.html">DOM based
37+
XSS Prevention Cheat Sheet</a>.
38+
</li>
39+
<li>
40+
OWASP:
41+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html">XSS
42+
(Cross Site Scripting) Prevention Cheat Sheet</a>.
43+
</li>
44+
<li>
45+
OWASP
46+
<a href="https://www.owasp.org/index.php/DOM_Based_XSS">DOM Based XSS</a>.
47+
</li>
48+
<li>
49+
OWASP
50+
<a href="https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting">Types of Cross-Site
51+
Scripting</a>.
52+
</li>
53+
<li>
54+
Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
55+
</li>
56+
</references>
57+
</qhelp>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Client-side cross-site scripting
3+
* @description Writing user input directly to the DOM allows for
4+
* a cross-site scripting vulnerability.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 6.1
8+
* @precision high
9+
* @id js/xss
10+
* @tags security
11+
* external/cwe/cwe-079
12+
* external/cwe/cwe-116
13+
*/
14+
15+
import javascript
16+
import semmle.javascript.security.dataflow.DomBasedXssQuery
17+
import DataFlow::PathGraph
18+
import semmle.javascript.heuristics.AdditionalSources
19+
20+
from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
21+
where cfg.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink,
23+
sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(),
24+
"user-provided value"
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
If a database query (such as a SQL or NoSQL query) is built from
9+
user-provided data without sufficient sanitization, a malicious user
10+
may be able to run malicious database queries.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Most database connector libraries offer a way of safely
17+
embedding untrusted data into a query by means of query parameters
18+
or prepared statements.
19+
</p>
20+
<p>
21+
For NoSQL queries, make use of an operator like MongoDB's <code>$eq</code>
22+
to ensure that untrusted data is interpreted as a literal value and not as
23+
a query object.
24+
</p>
25+
</recommendation>
26+
27+
<example>
28+
<p>
29+
In the following example, assume the function <code>handler</code> is
30+
an HTTP request handler in a web application, whose parameter
31+
<code>req</code> contains the request object.
32+
</p>
33+
34+
<p>
35+
The handler constructs two copies of the same SQL query involving
36+
user input taken from the request object, once unsafely using
37+
string concatenation, and once safely using query parameters.
38+
</p>
39+
40+
<p>
41+
In the first case, the query string <code>query1</code> is built by
42+
directly concatenating a user-supplied request parameter with some
43+
string literals. The parameter may include quote characters, so this
44+
code is vulnerable to a SQL injection attack.
45+
</p>
46+
47+
<p>
48+
In the second case, the parameter is embedded into the query string
49+
<code>query2</code> using query parameters. In this example, we use
50+
the API offered by the <code>pg</code> Postgres database connector
51+
library, but other libraries offer similar features. This version is
52+
immune to injection attacks.
53+
</p>
54+
55+
<sample src="examples/SqlInjection.js" />
56+
</example>
57+
58+
<references>
59+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
60+
<li>MongoDB: <a href="https://docs.mongodb.com/manual/reference/operator/query/eq">$eq operator</a>.</li>
61+
</references>
62+
</qhelp>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* @name Database query built from user-controlled sources
3+
* @description Building a database query from user-controlled sources is vulnerable to insertion of
4+
* malicious code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id js/sql-injection
10+
* @tags security
11+
* external/cwe/cwe-089
12+
* external/cwe/cwe-090
13+
* external/cwe/cwe-943
14+
*/
15+
16+
import javascript
17+
import semmle.javascript.security.dataflow.SqlInjectionQuery as SqlInjection
18+
import semmle.javascript.security.dataflow.NosqlInjectionQuery as NosqlInjection
19+
import DataFlow::PathGraph
20+
import semmle.javascript.heuristics.AdditionalSources
21+
22+
from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
23+
where
24+
(
25+
cfg instanceof SqlInjection::Configuration or
26+
cfg instanceof NosqlInjection::Configuration
27+
) and
28+
cfg.hasFlowPath(source, sink)
29+
select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
30+
"user-provided value"
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Directly evaluating user input (for example, an HTTP request parameter) as code without properly
9+
sanitizing the input first allows an attacker arbitrary code execution. This can occur when user
10+
input is treated as JavaScript, or passed to a framework which interprets it as an expression to be
11+
evaluated. Examples include AngularJS expressions or JQuery selectors.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Avoid including user input in any expression which may be dynamically evaluated. If user input must
18+
be included, use context-specific escaping before
19+
including it. It is important that the correct escaping is used for the type of evaluation that will
20+
occur.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
<p>
26+
The following example shows part of the page URL being evaluated as JavaScript code. This allows an
27+
attacker to provide JavaScript within the URL. If an attacker can persuade a user to click on a link
28+
to such a URL, the attacker can evaluate arbitrary JavaScript in the browser of the user to,
29+
for example, steal cookies containing session information.
30+
</p>
31+
32+
<sample src="examples/CodeInjection.js" />
33+
34+
<p>
35+
The following example shows a Pug template being constructed from user input, allowing attackers to run
36+
arbitrary code via a payload such as <code>#{global.process.exit(1)}</code>.
37+
</p>
38+
39+
<sample src="examples/ServerSideTemplateInjection.js" />
40+
41+
<p>
42+
Below is an example of how to use a template engine without any risk of template injection.
43+
The user input is included via an interpolation expression <code>#{username}</code> whose value is provided
44+
as an option to the template, instead of being part of the template string itself:
45+
</p>
46+
47+
<sample src="examples/ServerSideTemplateInjectionSafe.js" />
48+
</example>
49+
50+
<references>
51+
<li>
52+
OWASP:
53+
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
54+
</li>
55+
<li>
56+
Wikipedia: <a href="https://en.wikipedia.org/wiki/Code_injection">Code Injection</a>.
57+
</li>
58+
<li>
59+
PortSwigger Research Blog:
60+
<a href="https://portswigger.net/research/server-side-template-injection">Server-Side Template Injection</a>.
61+
</li>
62+
</references>
63+
</qhelp>

0 commit comments

Comments
 (0)
0