Skip to content

Commit 01163bc

Browse files
jheliejhelie
committed
ATM: add XSSThroughDOM boosted query
1 parent e01cbb2 commit 01163bc

2 files changed

Lines changed: 183 additions & 0 deletions

File tree

‎javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssThroughDomATM.qll‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/**
2+
* For internal use only.
3+
*
4+
* Defines shared code used by the XSS Through DOM boosted query.
5+
*/
6+
7+
private import semmle.javascript.heuristics.SyntacticHeuristics
8+
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
9+
private import semmle.javascript.dataflow.InferredTypes
10+
private import semmle.javascript.security.dataflow.XssThroughDomCustomizations::XssThroughDom as XssThroughDom
11+
private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQuery
12+
import AdaptiveThreatModeling
13+
import CoreKnowledge as CoreKnowledge
14+
import StandardEndpointFilters as StandardEndpointFilters
15+
16+
/**
17+
* This module provides logic to filter candidate sinks to those which are likely XSS sinks.
18+
*/
19+
module SinkEndpointFilter {
20+
/**
21+
* Provides a set of reasons why a given data flow node should be excluded as a sink candidate.
22+
*
23+
* If this predicate has no results for a sink candidate `n`, then we should treat `n` as an
24+
* effective sink.
25+
*/
26+
string getAReasonSinkExcluded(DataFlow::Node sinkCandidate) {
27+
result = StandardEndpointFilters::getAReasonSinkExcluded(sinkCandidate)
28+
or
29+
exists(DataFlow::CallNode call | sinkCandidate = call.getAnArgument() |
30+
call.getCalleeName() = "setState"
31+
) and
32+
result = "setState calls ought to be safe in react applications"
33+
or
34+
// Require XSS sink candidates to be (a) arguments to external library calls (possibly
35+
// indirectly), or (b) heuristic sinks.
36+
//
37+
// Heuristic sinks are copied from the `HeuristicDomBasedXssSink` class defined within
38+
// `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
39+
// We can't reuse the class because importing that file would cause us to treat these
40+
// heuristic sinks as known sinks.
41+
not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(sinkCandidate) and
42+
not (
43+
isAssignedToOrConcatenatedWith(sinkCandidate, "(?i)(html|innerhtml)")
44+
or
45+
isArgTo(sinkCandidate, "(?i)(html|render)")
46+
or
47+
sinkCandidate instanceof StringOps::HtmlConcatenationLeaf
48+
or
49+
isConcatenatedWithStrings("(?is).*<[a-z ]+.*", sinkCandidate, "(?s).*>.*")
50+
or
51+
// In addition to the heuristic sinks from `HeuristicDomBasedXssSink`, explicitly allow
52+
// property writes like `elem.innerHTML = <TAINT>` that may not be picked up as HTML
53+
// concatenation leaves.
54+
exists(DataFlow::PropWrite pw |
55+
pw.getPropertyName().regexpMatch("(?i).*html*") and
56+
pw.getRhs() = sinkCandidate
57+
)
58+
) and
59+
result = "not a direct argument to a likely external library call or a heuristic sink"
60+
}
61+
}
62+
63+
class XssThroughDomAtmConfig extends AtmConfig {
64+
XssThroughDomAtmConfig() { this = "XssThroughDomAtmConfig" }
65+
66+
override predicate isKnownSource(DataFlow::Node source) {
67+
source instanceof XssThroughDom::Source
68+
}
69+
70+
override predicate isEffectiveSink(DataFlow::Node sinkCandidate) {
71+
not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate))
72+
}
73+
74+
override EndpointType getASinkEndpointType() { result instanceof XssSinkType }
75+
}
76+
77+
/**
78+
* A taint-tracking configuration for reasoning about XSS through the DOM.
79+
*/
80+
class Configuration extends TaintTracking::Configuration {
81+
Configuration() { this = "XssThroughDomAtmConfig" }
82+
83+
override predicate isSource(DataFlow::Node source) { source instanceof XssThroughDom::Source }
84+
85+
override predicate isSink(DataFlow::Node sink) {
86+
sink instanceof DomBasedXss::Sink or
87+
any(XssThroughDomAtmConfig cfg).isEffectiveSink(sink)
88+
}
89+
90+
override predicate isSanitizer(DataFlow::Node node) {
91+
super.isSanitizer(node) or
92+
node instanceof DomBasedXss::Sanitizer
93+
}
94+
95+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
96+
guard instanceof TypeTestGuard or
97+
guard instanceof UnsafeJQuery::PropertyPresenceSanitizer or
98+
guard instanceof UnsafeJQuery::NumberGuard or
99+
guard instanceof PrefixStringSanitizer or
100+
guard instanceof QuoteGuard or
101+
guard instanceof ContainsHtmlGuard
102+
}
103+
104+
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
105+
DomBasedXss::isOptionallySanitizedEdge(pred, succ)
106+
}
107+
}
108+
109+
/**
110+
* A test of form `typeof x === "something"`, preventing `x` from being a string in some cases.
111+
*
112+
* This sanitizer helps prune infeasible paths in type-overloaded functions.
113+
*/
114+
class TypeTestGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
115+
override EqualityTest astNode;
116+
Expr operand;
117+
boolean polarity;
118+
119+
TypeTestGuard() {
120+
exists(TypeofTag tag | TaintTracking::isTypeofGuard(astNode, operand, tag) |
121+
// typeof x === "string" sanitizes `x` when it evaluates to false
122+
tag = "string" and
123+
polarity = astNode.getPolarity().booleanNot()
124+
or
125+
// typeof x === "object" sanitizes `x` when it evaluates to true
126+
tag != "string" and
127+
polarity = astNode.getPolarity()
128+
)
129+
}
130+
131+
override predicate sanitizes(boolean outcome, Expr e) {
132+
polarity = outcome and
133+
e = operand
134+
}
135+
}
136+
137+
private import semmle.javascript.security.dataflow.Xss::Shared as Shared
138+
139+
private class PrefixStringSanitizer extends TaintTracking::SanitizerGuardNode,
140+
DomBasedXss::PrefixStringSanitizer {
141+
PrefixStringSanitizer() { this = this }
142+
}
143+
144+
private class PrefixString extends DataFlow::FlowLabel, DomBasedXss::PrefixString {
145+
PrefixString() { this = this }
146+
}
147+
148+
private class QuoteGuard extends TaintTracking::SanitizerGuardNode, Shared::QuoteGuard {
149+
QuoteGuard() { this = this }
150+
}
151+
152+
private class ContainsHtmlGuard extends TaintTracking::SanitizerGuardNode, Shared::ContainsHtmlGuard {
153+
ContainsHtmlGuard() { this = this }
154+
}

‎javascript/ql/experimental/adaptivethreatmodeling/src/XssThroughDomATM.ql‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* For internal use only.
3+
*
4+
* @name DOM text reinterpreted as HTML (experimental)
5+
* @description Reinterpreting text from the DOM as HTML can lead
6+
* to a cross-site scripting vulnerability.
7+
* @kind path-problem
8+
* @scored
9+
* @problem.severity error
10+
* @security-severity 6.1
11+
* @id js/ml-powered/xss-through-dom
12+
* @tags experimental security
13+
* external/cwe/cwe-079 external/cwe/cwe-116
14+
*/
15+
16+
import javascript
17+
import ATM::ResultsInfo
18+
import DataFlow::PathGraph
19+
import experimental.adaptivethreatmodeling.XssThroughDomATM
20+
21+
from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
22+
where
23+
cfg.hasFlowPath(source, sink) and
24+
not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
25+
score = getScoreForFlow(source.getNode(), sink.getNode()) and
26+
score = getScoreForFlow(source.getNode(), sink.getNode())
27+
select sink.getNode(), source, sink,
28+
"(Experimental) $@ may be reinterpreted as HTML without escaping meta-characters. Identified using machine learning.",
29+
source.getNode(), "DOM text", score

0 commit comments

Comments
 (0)