Skip to content

Commit 7137a64

Browse files
erik-krogherik-krogh
committed
Added query for detecting XSS that happens through an exception
1 parent 1159344 commit 7137a64

File tree

9 files changed

+1662
-13
lines changed

9 files changed

+1662
-13
lines changed

‎javascript/ql/src/Security/CWE-079/ExceptionXss.qhelp‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Directly writing exceptions to a webpage with sanitization allows for a cross-site scripting
9+
vulnerability if the value of the exception can be influenzed by a user.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
To guard against cross-site scripting, consider using contextual output encoding/escaping before
16+
writing user input to the page, or one of the other solutions that are mentioned in the
17+
references.
18+
</p>
19+
</recommendation>
20+
21+
<example>
22+
<p>
23+
The following example shows an exception being written directly to the document,
24+
and this exception can potentially be influenzed the page URL,
25+
leaving the website vulnerable to cross-site scripting.
26+
</p>
27+
<sample src="examples/ExceptionXss.js" />
28+
</example>
29+
30+
<references>
31+
<li>
32+
OWASP:
33+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/DOM_based_XSS_Prevention_Cheat_Sheet.html">DOM based
34+
XSS Prevention Cheat Sheet</a>.
35+
</li>
36+
<li>
37+
OWASP:
38+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html">XSS
39+
(Cross Site Scripting) Prevention Cheat Sheet</a>.
40+
</li>
41+
<li>
42+
OWASP
43+
<a href="https://www.owasp.org/index.php/DOM_Based_XSS">DOM Based XSS</a>.
44+
</li>
45+
<li>
46+
OWASP
47+
<a href="https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting">Types of Cross-Site
48+
Scripting</a>.
49+
</li>
50+
<li>
51+
Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
52+
</li>
53+
</references>
54+
</qhelp>

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @name Client-side cross-site scripting through exception
3+
* @description User input being part of an exception allows for
4+
* cross-site scripting if that exception ends as input
5+
* to the DOM.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @precision low
9+
* @id js/xss-through-exception
10+
* @tags security
11+
* external/cwe/cwe-079
12+
* external/cwe/cwe-116
13+
*/
14+
15+
import javascript
16+
import semmle.javascript.security.dataflow.ExceptionXss::ExceptionXss
17+
import DataFlow::PathGraph
18+
19+
from
20+
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
21+
where
22+
cfg.hasFlowPath(source, sink) and
23+
not any(ConfigurationNoException c).hasFlow(source.getNode(), sink.getNode())
24+
select sink.getNode(), source, sink,
25+
sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(),
26+
"user-provided value"

‎javascript/ql/src/Security/CWE-079/examples/ExceptionXss.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
function setLanguageOptions() {
2+
var href = document.location.href,
3+
deflt = href.substring(href.indexOf("default=")+8);
4+
5+
try {
6+
var parsed = unknownParseFunction(deflt);
7+
} catch(e) {
8+
document.write("Had an error: " + e + ".");
9+
}
10+
}

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import javascript
77

88
module DomBasedXss {
9-
import Xss::DomBasedXss
9+
import DomBasedXssCustomizations::DomBasedXss
1010

1111
/**
1212
* A taint-tracking configuration for reasoning about XSS.
@@ -33,16 +33,4 @@ module DomBasedXss {
3333
node instanceof Sanitizer
3434
}
3535
}
36-
37-
/** A source of remote user input, considered as a flow source for DOM-based XSS. */
38-
class RemoteFlowSourceAsSource extends Source {
39-
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
40-
}
41-
42-
/**
43-
* An access of the URL of this page, or of the referrer to this page.
44-
*/
45-
class LocationSource extends Source {
46-
LocationSource() { this = DOM::locationSource() }
47-
}
4836
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* Provides default sources for reasoning about DOM-based
3+
* cross-site scripting vulnerabilities.
4+
*/
5+
6+
7+
import javascript
8+
9+
module DomBasedXss {
10+
import Xss::DomBasedXss
11+
12+
/** A source of remote user input, considered as a flow source for DOM-based XSS. */
13+
class RemoteFlowSourceAsSource extends Source {
14+
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
15+
}
16+
17+
/**
18+
* An access of the URL of this page, or of the referrer to this page.
19+
*/
20+
class LocationSource extends Source {
21+
LocationSource() { this = DOM::locationSource() }
22+
}
23+
}

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

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* Provides a taint-tracking configuration for TODO:
3+
*/
4+
5+
import javascript
6+
7+
module ExceptionXss {
8+
import Xss::DomBasedXss // imports sinks
9+
import DomBasedXssCustomizations::DomBasedXss // imports sources
10+
11+
DataFlow::Node getExceptionalSuccssor(DataFlow::Node pred) {
12+
exists(DataFlow::FunctionNode func |
13+
pred.getContainer() = func.getFunction() and
14+
if exists(getEnclosingTryStmt(pred.asExpr().getEnclosingStmt()))
15+
then
16+
result.(DataFlow::ParameterNode).getParameter() = getEnclosingTryStmt(pred
17+
.asExpr()
18+
.getEnclosingStmt()).getACatchClause().getAParameter()
19+
else result = getExceptionalSuccssor(func.getExceptionalReturn())
20+
or
21+
pred = func.getExceptionalReturn() and
22+
exists(DataFlow::InvokeNode call |
23+
not call.isImprecise() and
24+
func.getFunction() = call.(DataFlow::InvokeNode).getACallee() and
25+
result = getExceptionalSuccssor(call)
26+
)
27+
)
28+
}
29+
30+
predicate canThrowSensitiveInformation(DataFlow::Node node) {
31+
// i have to do this "dual" check because there are two data-flow nodes associated with reflective calls.
32+
node = any(DataFlow::InvokeNode call).getAnArgument() and
33+
not node = any(DataFlow::InvokeNode call | exists(call.getACallee())).getAnArgument()
34+
or
35+
node.asExpr().getEnclosingStmt() instanceof ThrowStmt
36+
or
37+
// TODO: Do some flow label for deeply tainted objects?
38+
exists(DataFlow::ObjectLiteralNode obj |
39+
obj.asExpr().(ObjectExpr).getAProperty().getInit() = node.asExpr() and
40+
canThrowSensitiveInformation(obj)
41+
)
42+
or
43+
exists(DataFlow::ArrayCreationNode arr |
44+
arr.getAnElement() = node and
45+
canThrowSensitiveInformation(arr)
46+
)
47+
}
48+
49+
TryStmt getEnclosingTryStmt(Stmt stmt) {
50+
stmt.getParentStmt+() = result.getBody() and
51+
not exists(TryStmt mid |
52+
stmt.getParentStmt+() = mid.getBody() and mid.getParentStmt+() = result.getBody()
53+
)
54+
}
55+
56+
/**
57+
* A taint-tracking configuration for reasoning about XSS with possible exceptional flow.
58+
*/
59+
abstract class BaseConfiguration extends TaintTracking::Configuration {
60+
BaseConfiguration() { this = "ExceptionXss" or this = "ExceptionXssNoException" }
61+
62+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
63+
64+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
65+
66+
override predicate isSanitizer(DataFlow::Node node) {
67+
super.isSanitizer(node) or
68+
node instanceof Sanitizer
69+
}
70+
71+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
72+
succ = getExceptionalSuccssor(pred) and
73+
(
74+
canThrowSensitiveInformation(pred)
75+
or
76+
pred = any(DataFlow::FunctionNode func).getExceptionalReturn()
77+
)
78+
or
79+
// String.prototype.match()
80+
exists(DataFlow::MethodCallNode call |
81+
call = succ and
82+
pred = call.getReceiver() and
83+
call.getMethodName() = "match" and
84+
call.getNumArgument() = 1 and
85+
// TODO: Better way of detecting regExp / String.prototype.match() calls?
86+
(
87+
call.getArgument(0).getALocalSource().asExpr() instanceof RegExpLiteral or
88+
call.getArgument(0).getALocalSource().(DataFlow::NewNode).getCalleeName() = "RegExp"
89+
)
90+
)
91+
}
92+
}
93+
94+
/**
95+
* Instantiation of BaseConfiguration.
96+
*/
97+
class Configuration extends BaseConfiguration {
98+
Configuration() { this = "ExceptionXss" }
99+
}
100+
101+
/**
102+
* Same as configuration, except that all additional exceptional flows has been removed.
103+
*/
104+
class ConfigurationNoException extends BaseConfiguration {
105+
ConfigurationNoException() { this = "ExceptionXssNoException" }
106+
107+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
108+
super.isAdditionalTaintStep(pred, succ) and
109+
not succ = getExceptionalSuccssor(pred)
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)