Skip to content

Commit 7e947b2

Browse files
asgerfasgerf
committed
JS: Use return value of trusted type policy callback as a sink
1 parent 9ffa236 commit 7e947b2

File tree

5 files changed

+98
-0
lines changed

5 files changed

+98
-0
lines changed

‎javascript/ql/lib/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ import semmle.javascript.frameworks.SocketIO
126126
import semmle.javascript.frameworks.StringFormatters
127127
import semmle.javascript.frameworks.TorrentLibraries
128128
import semmle.javascript.frameworks.Typeahead
129+
import semmle.javascript.frameworks.TrustedTypes
129130
import semmle.javascript.frameworks.UriLibraries
130131
import semmle.javascript.frameworks.Vue
131132
import semmle.javascript.frameworks.Vuex

‎javascript/ql/lib/semmle/javascript/frameworks/TrustedTypes.qll

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/** Module for working with uses of the Trusted Types API. */
2+
3+
private import javascript
4+
private import semmle.javascript.security.dataflow.Xss
5+
private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations
6+
private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
7+
8+
/** Module for working with uses of the Trusted Types API. */
9+
module TrustedTypes {
10+
private class TrustedTypesEntry extends API::EntryPoint {
11+
TrustedTypesEntry() { this = "TrustedTypesEntry" }
12+
13+
override DataFlow::SourceNode getAUse() { result = DataFlow::globalVarRef("trustedTypes") }
14+
15+
override DataFlow::Node getARhs() { none() }
16+
}
17+
18+
private API::Node trustedTypesObj() { result = any(TrustedTypesEntry entry).getNode() }
19+
20+
/** A call to `trustedTypes.createPolicy`. */
21+
class PolicyCreation extends API::CallNode {
22+
PolicyCreation() { this = trustedTypesObj().getMember("createPolicy").getACall() }
23+
24+
DataFlow::FunctionNode getPolicyCallback(string method) {
25+
// Require local callback to avoid potential call/return mismatch in the uses below
26+
result = getOptionArgument(1, method).getALocalSource()
27+
}
28+
}
29+
30+
/**
31+
* A data-flow step from the use of a policy to its callback.
32+
*/
33+
private class PolicyInputStep extends DataFlow::SharedFlowStep {
34+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
35+
exists(PolicyCreation policy, string method |
36+
pred = policy.getReturn().getMember(method).getParameter(0).getARhs() and
37+
succ = policy.getPolicyCallback(method).getParameter(0)
38+
)
39+
}
40+
}
41+
42+
/**
43+
* The creation of a trusted HTML object, as an XSS sink.
44+
*/
45+
private class XssSink extends DomBasedXss::Sink {
46+
XssSink() { this = any(PolicyCreation creation).getPolicyCallback("createHTML").getAReturn() }
47+
}
48+
49+
/**
50+
* The creation of a trusted script, as a code-injection sink.
51+
*/
52+
private class CodeInjectionSink extends CodeInjection::Sink {
53+
CodeInjectionSink() {
54+
this = any(PolicyCreation creation).getPolicyCallback("createScript").getAReturn()
55+
}
56+
}
57+
58+
/**
59+
* The creation of a trusted script URL, as a URL redirection sink.
60+
*
61+
* This is currently handled by the client-side URL redirection query, as this checks for untrusted hostname in the URL.
62+
*/
63+
private class UrlSink extends ClientSideUrlRedirect::Sink {
64+
UrlSink() {
65+
this = any(PolicyCreation creation).getPolicyCallback("createScriptURL").getAReturn()
66+
}
67+
}
68+
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,11 @@ nodes
427427
| translate.js:7:42:7:60 | target.substring(1) |
428428
| translate.js:9:27:9:50 | searchP ... 'term') |
429429
| translate.js:9:27:9:50 | searchP ... 'term') |
430+
| trusted-types.js:2:66:2:66 | x |
431+
| trusted-types.js:2:71:2:71 | x |
432+
| trusted-types.js:2:71:2:71 | x |
433+
| trusted-types.js:3:24:3:34 | window.name |
434+
| trusted-types.js:3:24:3:34 | window.name |
430435
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) |
431436
| tst3.js:2:23:2:74 | decodeU ... str(1)) |
432437
| tst3.js:2:42:2:63 | window. ... .search |
@@ -1183,6 +1188,10 @@ edges
11831188
| translate.js:7:42:7:47 | target | translate.js:7:42:7:60 | target.substring(1) |
11841189
| translate.js:7:42:7:60 | target.substring(1) | translate.js:9:27:9:50 | searchP ... 'term') |
11851190
| translate.js:7:42:7:60 | target.substring(1) | translate.js:9:27:9:50 | searchP ... 'term') |
1191+
| trusted-types.js:2:66:2:66 | x | trusted-types.js:2:71:2:71 | x |
1192+
| trusted-types.js:2:66:2:66 | x | trusted-types.js:2:71:2:71 | x |
1193+
| trusted-types.js:3:24:3:34 | window.name | trusted-types.js:2:66:2:66 | x |
1194+
| trusted-types.js:3:24:3:34 | window.name | trusted-types.js:2:66:2:66 | x |
11861195
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:4:25:4:28 | data |
11871196
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:5:26:5:29 | data |
11881197
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:7:32:7:35 | data |
@@ -1612,6 +1621,7 @@ edges
16121621
| tooltip.jsx:10:25:10:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:10:25:10:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
16131622
| tooltip.jsx:11:25:11:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:11:25:11:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
16141623
| translate.js:9:27:9:50 | searchP ... 'term') | translate.js:6:16:6:39 | documen ... .search | translate.js:9:27:9:50 | searchP ... 'term') | Cross-site scripting vulnerability due to $@. | translate.js:6:16:6:39 | documen ... .search | user-provided value |
1624+
| trusted-types.js:2:71:2:71 | x | trusted-types.js:3:24:3:34 | window.name | trusted-types.js:2:71:2:71 | x | Cross-site scripting vulnerability due to $@. | trusted-types.js:3:24:3:34 | window.name | user-provided value |
16151625
| tst3.js:4:25:4:32 | data.src | tst3.js:2:42:2:63 | window. ... .search | tst3.js:4:25:4:32 | data.src | Cross-site scripting vulnerability due to $@. | tst3.js:2:42:2:63 | window. ... .search | user-provided value |
16161626
| tst3.js:5:26:5:31 | data.p | tst3.js:2:42:2:63 | window. ... .search | tst3.js:5:26:5:31 | data.p | Cross-site scripting vulnerability due to $@. | tst3.js:2:42:2:63 | window. ... .search | user-provided value |
16171627
| tst3.js:7:32:7:37 | data.p | tst3.js:2:42:2:63 | window. ... .search | tst3.js:7:32:7:37 | data.p | Cross-site scripting vulnerability due to $@. | tst3.js:2:42:2:63 | window. ... .search | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,11 @@ nodes
434434
| translate.js:7:42:7:60 | target.substring(1) |
435435
| translate.js:9:27:9:50 | searchP ... 'term') |
436436
| translate.js:9:27:9:50 | searchP ... 'term') |
437+
| trusted-types.js:2:66:2:66 | x |
438+
| trusted-types.js:2:71:2:71 | x |
439+
| trusted-types.js:2:71:2:71 | x |
440+
| trusted-types.js:3:24:3:34 | window.name |
441+
| trusted-types.js:3:24:3:34 | window.name |
437442
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) |
438443
| tst3.js:2:23:2:74 | decodeU ... str(1)) |
439444
| tst3.js:2:42:2:63 | window. ... .search |
@@ -1218,6 +1223,10 @@ edges
12181223
| translate.js:7:42:7:47 | target | translate.js:7:42:7:60 | target.substring(1) |
12191224
| translate.js:7:42:7:60 | target.substring(1) | translate.js:9:27:9:50 | searchP ... 'term') |
12201225
| translate.js:7:42:7:60 | target.substring(1) | translate.js:9:27:9:50 | searchP ... 'term') |
1226+
| trusted-types.js:2:66:2:66 | x | trusted-types.js:2:71:2:71 | x |
1227+
| trusted-types.js:2:66:2:66 | x | trusted-types.js:2:71:2:71 | x |
1228+
| trusted-types.js:3:24:3:34 | window.name | trusted-types.js:2:66:2:66 | x |
1229+
| trusted-types.js:3:24:3:34 | window.name | trusted-types.js:2:66:2:66 | x |
12211230
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:4:25:4:28 | data |
12221231
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:5:26:5:29 | data |
12231232
| tst3.js:2:12:2:75 | JSON.pa ... tr(1))) | tst3.js:7:32:7:35 | data |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/trusted-types.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
(function() {
2+
const policy1 = trustedTypes.createPolicy('x', { createHTML: x => x }); // NOT OK
3+
policy1.createHTML(window.name);
4+
5+
const policy2 = trustedTypes.createPolicy('x', { createHTML: x => 'safe' }); // OK
6+
policy2.createHTML(window.name);
7+
8+
const policy3 = trustedTypes.createPolicy('x', { createHTML: x => x }); // OK
9+
policy3.createHTML('safe');
10+
})();

0 commit comments

Comments
 (0)