Skip to content

Commit c08230c

Browse files
authored
Merge pull request github#5378 from asgerf/js/meta-problem-queries
Approved by esbena
2 parents 2414019 + 3fb810b commit c08230c

File tree

10 files changed

+126
-22
lines changed

10 files changed

+126
-22
lines changed
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* @name Call graph
3+
* @description An edge in the call graph.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id js/meta/alerts/call-graph
7+
* @tags meta
8+
* @precision very-low
9+
*/
10+
11+
import javascript
12+
13+
from DataFlow::InvokeNode invoke, Function f
14+
where invoke.getACallee() = f
15+
select invoke, "Call to $@", f, f.describe()
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* @name Taint sinks
3+
* @description Sinks that are sensitive to untrusted data.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id js/meta/alerts/taint-sinks
7+
* @tags meta
8+
* @precision very-low
9+
*/
10+
11+
import javascript
12+
import meta.internal.TaintMetrics
13+
14+
from string kind
15+
select relevantTaintSink(kind), kind + " sink"
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Taint sources
3+
* @description Sources of untrusted input.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @id js/meta/alerts/taint-sources
7+
* @tags meta
8+
* @precision very-low
9+
*/
10+
11+
import javascript
12+
import meta.internal.TaintMetrics
13+
14+
string getName(DataFlow::Node node) {
15+
result = node.(RemoteFlowSource).getSourceType()
16+
or
17+
not node instanceof RemoteFlowSource and
18+
result = "Taint source"
19+
}
20+
21+
from DataFlow::Node node
22+
where node = relevantTaintSource()
23+
select node, getName(node)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* @name Tainted expressions
3+
* @description The number of expressions reachable from a remote flow source
4+
* via default taint-tracking steps.
5+
* @kind problem
6+
* @problem.severity recommendation
7+
* @tags meta
8+
* @id js/meta/alerts/tainted-nodes
9+
* @precision very-low
10+
*/
11+
12+
import javascript
13+
import meta.internal.TaintMetrics
14+
15+
class BasicTaintConfiguration extends TaintTracking::Configuration {
16+
BasicTaintConfiguration() { this = "BasicTaintConfiguration" }
17+
18+
override predicate isSource(DataFlow::Node node) { node = relevantTaintSource() }
19+
20+
override predicate isSink(DataFlow::Node node) {
21+
// To reduce noise from synthetic nodes, only count value nodes
22+
node instanceof DataFlow::ValueNode and
23+
not node.getFile() instanceof IgnoredFile
24+
}
25+
}
26+
27+
// Avoid linking to the source as this would upset the statistics: nodes reachable
28+
// from multiple sources would be counted multilpe times, and that's not what we intend to measure.
29+
from DataFlow::Node node
30+
where any(BasicTaintConfiguration cfg).hasFlow(_, node)
31+
select node, "Tainted node"

javascript/ql/src/meta/analysis-quality/SanitizersReachableFromSource.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010

1111
import javascript
12-
import TaintMetrics
12+
import meta.internal.TaintMetrics
1313

1414
class BasicTaintConfiguration extends TaintTracking::Configuration {
1515
BasicTaintConfiguration() { this = "BasicTaintConfiguration" }

javascript/ql/src/meta/analysis-quality/SinksReachableFromSanitizer.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010

1111
import javascript
12-
import TaintMetrics
12+
import meta.internal.TaintMetrics
1313

1414
class BasicTaintConfiguration extends TaintTracking::Configuration {
1515
BasicTaintConfiguration() { this = "BasicTaintConfiguration" }

javascript/ql/src/meta/analysis-quality/TaintSinks.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99
*/
1010

1111
import javascript
12-
import TaintMetrics
12+
import meta.internal.TaintMetrics
1313

1414
select projectRoot(), count(relevantTaintSink())

javascript/ql/src/meta/analysis-quality/TaintSources.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99
*/
1010

1111
import javascript
12-
import TaintMetrics
12+
import meta.internal.TaintMetrics
1313

1414
select projectRoot(), count(relevantTaintSource())

javascript/ql/src/meta/analysis-quality/TaintedNodes.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*/
1111

1212
import javascript
13-
import TaintMetrics
13+
import meta.internal.TaintMetrics
1414

1515
class BasicTaintConfiguration extends TaintTracking::Configuration {
1616
BasicTaintConfiguration() { this = "BasicTaintConfiguration" }

javascript/ql/src/meta/analysis-quality/TaintMetrics.qll renamed to javascript/ql/src/meta/internal/TaintMetrics.qll

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,49 @@ private import semmle.javascript.security.dataflow.ZipSlipCustomizations
3131
* Examples of excluded queries:
3232
* - UnsafeDynamicMethodAccess: high severity (RCE) but has way too many sinks (every callee).
3333
* - ClearTextLogging: not severe enough relative to number of sinks.
34+
*
35+
* `kind` is bound to the name of the module containing the query sinks.
3436
*/
35-
DataFlow::Node relevantTaintSink() {
37+
DataFlow::Node relevantTaintSink(string kind) {
3638
not result.getFile() instanceof IgnoredFile and
3739
(
38-
result instanceof ClientSideUrlRedirect::Sink or
39-
result instanceof CodeInjection::Sink or
40-
result instanceof CommandInjection::Sink or
41-
result instanceof Xss::Shared::Sink or
42-
result instanceof NosqlInjection::Sink or
43-
result instanceof PrototypePollution::Sink or
44-
result instanceof RegExpInjection::Sink or
45-
result instanceof RequestForgery::Sink or
46-
result instanceof ServerSideUrlRedirect::Sink or
47-
result instanceof SqlInjection::Sink or
48-
result instanceof TaintedPath::Sink or
49-
result instanceof UnsafeDeserialization::Sink or
50-
result instanceof XmlBomb::Sink or
51-
result instanceof XpathInjection::Sink or
52-
result instanceof Xxe::Sink or
53-
result instanceof ZipSlip::Sink
40+
kind = "ClientSideUrlRedirect" and result instanceof ClientSideUrlRedirect::Sink
41+
or
42+
kind = "CodeInjection" and result instanceof CodeInjection::Sink
43+
or
44+
kind = "CommandInjection" and result instanceof CommandInjection::Sink
45+
or
46+
kind = "Xss" and result instanceof Xss::Shared::Sink
47+
or
48+
kind = "NosqlInjection" and result instanceof NosqlInjection::Sink
49+
or
50+
kind = "PrototypePollution" and result instanceof PrototypePollution::Sink
51+
or
52+
kind = "RegExpInjection" and result instanceof RegExpInjection::Sink
53+
or
54+
kind = "RequestForgery" and result instanceof RequestForgery::Sink
55+
or
56+
kind = "ServerSideUrlRedirect" and result instanceof ServerSideUrlRedirect::Sink
57+
or
58+
kind = "SqlInjection" and result instanceof SqlInjection::Sink
59+
or
60+
kind = "TaintedPath" and result instanceof TaintedPath::Sink
61+
or
62+
kind = "UnsafeDeserialization" and result instanceof UnsafeDeserialization::Sink
63+
or
64+
kind = "XmlBomb" and result instanceof XmlBomb::Sink
65+
or
66+
kind = "XpathInjection" and result instanceof XpathInjection::Sink
67+
or
68+
kind = "Xxe" and result instanceof Xxe::Sink
69+
or
70+
kind = "ZipSlip" and result instanceof ZipSlip::Sink
5471
)
5572
}
5673

74+
/** Gets a relevant taint sink. See `relevantTaintSink/1` for more information. */
75+
DataFlow::Node relevantTaintSink() { result = relevantTaintSink(_) }
76+
5777
/**
5878
* Gets a remote flow source or `document.location` source.
5979
*/

0 commit comments

Comments
 (0)