Skip to content

Commit e86a3b5

Browse files
erik-krogherik-krogh
committed
add js/html-constructed-from-input query
1 parent a400a1e commit e86a3b5

File tree

4 files changed

+225
-0
lines changed

4 files changed

+225
-0
lines changed

‎javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Unsafe HTML constructed from library input
3+
* @description Using externally controlled strings to construct HTML might allow a malicious
4+
* user to perform an cross-site scripting attack.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id js/html-constructed-from-input
9+
* @tags security
10+
* external/cwe/cwe-079
11+
* external/cwe/cwe-116
12+
*/
13+
14+
import javascript
15+
import DataFlow::PathGraph
16+
import semmle.javascript.security.dataflow.UnsafeHtmlConstruction::UnsafeHtmlConstruction
17+
18+
from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
19+
where cfg.hasFlowPath(source, sink) and sink.getNode() = sinkNode
20+
select sinkNode, source, sink, "$@ based on $@ might later cause $@.", sinkNode,
21+
sinkNode.describe(), source.getNode(), "library input", sinkNode.getSink(),
22+
sinkNode.getVulnerabilityKind().toLowerCase()

‎javascript/ql/src/semmle/javascript/frameworks/Markdown.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ module Markdown {
1313
* A taint-step that parses a markdown document, but preserves taint.import
1414
*/
1515
class MarkdownStep extends Unit {
16+
/**
17+
* Holds if there is a taint-step from `pred` to `succ` for a taint-preserving markdown parser.
18+
*/
1619
abstract predicate step(DataFlow::Node pred, DataFlow::Node succ);
1720
}
1821

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about
3+
* unsafe HTML constructed from library input vulnerabilities.
4+
*/
5+
6+
import javascript
7+
8+
/**
9+
* Classes and predicates for the unsafe HTML constructed from library input query.
10+
*/
11+
module UnsafeHtmlConstruction {
12+
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss
13+
private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin
14+
import UnsafeHtmlConstructionCustomizations::UnsafeHtmlConstruction
15+
16+
/**
17+
* A taint-tracking configuration for reasoning about unsafe HTML constructed from library input vulnerabilities.
18+
*/
19+
class Configration extends TaintTracking::Configuration {
20+
Configration() { this = "UnsafeHtmlConstruction" }
21+
22+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
25+
26+
override predicate isSanitizer(DataFlow::Node node) { super.isSanitizer(node) }
27+
28+
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
29+
DomBasedXss::isOptionallySanitizedEdge(pred, succ)
30+
}
31+
}
32+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* unsafe HTML constructed from library input, as well as extension points
4+
* for adding your own.
5+
*/
6+
7+
import javascript
8+
9+
/**
10+
* Module containing sources, sinks, and sanitizers for unsafe HTML constructed from library input.
11+
*/
12+
module UnsafeHtmlConstruction {
13+
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss
14+
private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin
15+
private import semmle.javascript.PackageExports as Exports
16+
17+
/**
18+
* A source for unsafe HTML constructed from library input.
19+
*/
20+
abstract class Source extends DataFlow::Node { }
21+
22+
/**
23+
* A parameter of an exported function, seen as a source for usnafe HTML constructed from input.
24+
*/
25+
class ExternalInputSource extends Source, DataFlow::ParameterNode {
26+
ExternalInputSource() {
27+
this = Exports::getALibraryInputParameter() and
28+
not this = JQuery::dollarSource()
29+
}
30+
}
31+
32+
/**
33+
* A sink for unsafe HTML constructed from library input.
34+
* This sink somehow transforms its input into a value that can cause XSS if it ends up in a XSS sink.
35+
*/
36+
abstract class Sink extends DataFlow::Node {
37+
/**
38+
* Gets the kind of vulnerability to report in the alert message.
39+
*
40+
* Defaults to `Cross-site scripting`, but may be overriden for sinks
41+
* that do not allow script injection, but injection of other undesirable HTML elements.
42+
*/
43+
abstract string getVulnerabilityKind();
44+
45+
/**
46+
* Gets the XSS sink that this transformed input ends up in.
47+
*/
48+
abstract DataFlow::Node getSink();
49+
50+
/**
51+
* Gets a string describing the transformation that this sink represents.
52+
*/
53+
abstract string describe();
54+
}
55+
56+
/**
57+
* A sink for `js/html-constructed-from-input` that constructs some HTML where
58+
* that HTML is later used in `xssSink`.
59+
*/
60+
abstract class XssSink extends Sink {
61+
DomBasedXss::Sink xssSink;
62+
63+
final override string getVulnerabilityKind() { result = xssSink.getVulnerabilityKind() }
64+
65+
final override DomBasedXss::Sink getSink() { result = xssSink }
66+
}
67+
68+
/**
69+
* Gets a dataflow node that flows to `sink` tracked by `t`.
70+
*/
71+
private DataFlow::Node isUsedInXssSink(DataFlow::TypeBackTracker t, DomBasedXss::Sink sink) {
72+
t.start() and
73+
result = sink
74+
or
75+
exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, isUsedInXssSink(t2, sink)))
76+
or
77+
exists(DataFlow::TypeBackTracker t2 |
78+
t.continue() = t2 and
79+
domBasedTaintStep(result, isUsedInXssSink(t2, sink))
80+
)
81+
}
82+
83+
/**
84+
* Gets a dataflow node that flows to `sink`.
85+
*/
86+
DataFlow::Node isUsedInXssSink(DomBasedXss::Sink sink) {
87+
result = isUsedInXssSink(DataFlow::TypeBackTracker::end(), sink)
88+
}
89+
90+
/**
91+
* Holds if there is a taint step from `pred` to `succ` for DOM strings/nodes.
92+
* These steps are mostly relevant for DOM nodes that are created by an XML parser.
93+
*/
94+
predicate domBasedTaintStep(DataFlow::Node pred, DataFlow::SourceNode succ) {
95+
// node.appendChild(newChild) and similar
96+
exists(DataFlow::MethodCallNode call |
97+
call.getMethodName() = ["insertBefore", "replaceChild", "appendChild"]
98+
|
99+
pred = call.getArgument(0) and
100+
succ = [call.getReceiver().getALocalSource(), call]
101+
)
102+
or
103+
// element.{prepend,append}(node) and similar
104+
exists(DataFlow::MethodCallNode call |
105+
call.getMethodName() = ["prepend", "append", "replaceWith", "replaceChildren"]
106+
|
107+
pred = call.getAnArgument() and
108+
succ = call.getReceiver().getALocalSource()
109+
)
110+
or
111+
// node.insertAdjacentElement("location", newChild)
112+
exists(DataFlow::MethodCallNode call | call.getMethodName() = "insertAdjacentElement" |
113+
pred = call.getArgument(1) and
114+
succ = call.getReceiver().getALocalSource()
115+
)
116+
or
117+
// clone = node.cloneNode()
118+
exists(DataFlow::MethodCallNode cloneNode | cloneNode.getMethodName() = "cloneNode" |
119+
pred = cloneNode.getReceiver() and
120+
succ = cloneNode
121+
)
122+
or
123+
// var succ = pred.documentElement;
124+
// documentElement is the root element of the document, and childNodes is the list of all children
125+
exists(DataFlow::PropRead read | read.getPropertyName() = ["documentElement", "childNodes"] |
126+
pred = read.getBase() and
127+
succ = read
128+
)
129+
}
130+
131+
/**
132+
* A string-concatenation of HTML, where the result is used as an XSS sink.
133+
*/
134+
class HTMLConcatenationSink extends XssSink, StringOps::HtmlConcatenationLeaf {
135+
HTMLConcatenationSink() { isUsedInXssSink(xssSink) = this.getRoot() }
136+
137+
override string describe() { result = "HTML construction" }
138+
}
139+
140+
/**
141+
* A string parsed as XML, which is later used in an XSS sink.
142+
*/
143+
class XMLParsedSink extends XssSink {
144+
XML::ParserInvocation parser;
145+
146+
XMLParsedSink() {
147+
this.asExpr() = parser.getSourceArgument() and
148+
isUsedInXssSink(xssSink) = parser.getAResult()
149+
}
150+
151+
override string describe() { result = "XML parsing" }
152+
}
153+
154+
/**
155+
* A string rendered as markdown, where the rendering preserves HTML.
156+
*/
157+
class MarkdownSink extends XssSink {
158+
MarkdownSink() {
159+
exists(DataFlow::Node pred, DataFlow::Node succ, Markdown::MarkdownStep step |
160+
step.step(pred, succ) and
161+
this = pred and
162+
succ = isUsedInXssSink(xssSink)
163+
)
164+
}
165+
166+
override string describe() { result = "Markdown rendering" }
167+
}
168+
}

0 commit comments

Comments
 (0)