diff --git a/change-notes/1.26/analysis-javascript.md b/change-notes/1.26/analysis-javascript.md index 8797c05cd42..9d6f2af5970 100644 --- a/change-notes/1.26/analysis-javascript.md +++ b/change-notes/1.26/analysis-javascript.md @@ -53,7 +53,9 @@ | Client-side URL redirect (`js/client-side-unvalidated-url-redirection`) | More results | This query now recognizes some unsafe uses of `importScripts()` inside WebWorkers. | | Missing CSRF middleware (`js/missing-token-validation`) | More results | This query now recognizes writes to cookie and session variables as potentially vulnerable to CSRF attacks. | | Missing CSRF middleware (`js/missing-token-validation`) | Fewer results | This query now recognizes more ways of protecting against CSRF attacks. | +| Client-side cross-site scripting (`js/xss`) | More results | This query now tracks data flow from `location.hash` more precisely. | ## Changes to libraries * The predicate `TypeAnnotation.hasQualifiedName` now works in more cases when the imported library was not present during extraction. +* The class `DomBasedXss::Configuration` has been deprecated, as it has been split into `DomBasedXss::HtmlInjectionConfiguration` and `DomBasedXss::JQueryHtmlOrSelectorInjectionConfiguration`. Unless specifically working with jQuery sinks, subclasses should instead be based on `HtmlInjectionConfiguration`. To use both configurations in a query, see [Xss.ql](https://github.com/github/codeql/blob/main/javascript/ql/src/Security/CWE-079/Xss.ql) for an example. diff --git a/javascript/ql/src/Security/CWE-079/Xss.ql b/javascript/ql/src/Security/CWE-079/Xss.ql index 29f66aac49e..3925febb008 100644 --- a/javascript/ql/src/Security/CWE-079/Xss.ql +++ b/javascript/ql/src/Security/CWE-079/Xss.ql @@ -15,8 +15,13 @@ import javascript import semmle.javascript.security.dataflow.DomBasedXss::DomBasedXss import DataFlow::PathGraph -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where + ( + cfg instanceof HtmlInjectionConfiguration or + cfg instanceof JQueryHtmlOrSelectorInjectionConfiguration + ) and + cfg.hasFlowPath(source, sink) select sink.getNode(), source, sink, sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(), "user-provided value" diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll index f332633dfbc..621e7acbc83 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll @@ -8,15 +8,23 @@ import javascript module DomBasedXss { import DomBasedXssCustomizations::DomBasedXss + /** + * DEPRECATED. Use `HtmlInjectionConfiguration` or `JQueryHtmlOrSelectorInjectionConfiguration`. + */ + deprecated class Configuration = HtmlInjectionConfiguration; + /** * A taint-tracking configuration for reasoning about XSS. */ - class Configuration extends TaintTracking::Configuration { - Configuration() { this = "DomBasedXss" } + class HtmlInjectionConfiguration extends TaintTracking::Configuration { + HtmlInjectionConfiguration() { this = "HtmlInjection" } override predicate isSource(DataFlow::Node source) { source instanceof Source } - override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + override predicate isSink(DataFlow::Node sink) { + sink instanceof Sink and + not sink instanceof JQueryHtmlOrSelectorSink // Handled by JQueryHtmlOrSelectorInjectionConfiguration below + } override predicate isSanitizer(DataFlow::Node node) { super.isSanitizer(node) @@ -28,59 +36,68 @@ module DomBasedXss { guard instanceof SanitizerGuard } - override predicate isAdditionalStoreStep( - DataFlow::Node pred, DataFlow::SourceNode succ, string prop - ) { - exists(DataFlow::PropRead read | - pred = read.getBase() and - succ = read and - read.getPropertyName() = "hash" and - prop = urlSuffixPseudoProperty() - ) - } - - override predicate isAdditionalLoadStoreStep( - DataFlow::Node pred, DataFlow::Node succ, string predProp, string succProp - ) { - exists(DataFlow::PropRead read | - pred = read.getBase() and - succ = read and - read.getPropertyName() = "hash" and - predProp = "hash" and - succProp = urlSuffixPseudoProperty() - ) - } - - override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { - exists(DataFlow::MethodCallNode call | - call.getMethodName() = ["substr", "substring", "slice"] and - not call.getArgument(0).getIntValue() = 0 and - pred = call.getReceiver() and - succ = call and - prop = urlSuffixPseudoProperty() - ) - or - exists(DataFlow::MethodCallNode call | - call.getMethodName() = "exec" and pred = call.getArgument(0) - or - call.getMethodName() = "match" and pred = call.getReceiver() - | - succ = call and - prop = urlSuffixPseudoProperty() - ) - or - exists(StringSplitCall split | - split.getSeparator() = ["#", "?"] and - pred = split.getBaseString() and - succ = split.getASubstringRead(1) and - prop = urlSuffixPseudoProperty() - ) - } - override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { DomBasedXss::isOptionallySanitizedEdge(pred, succ) } } - private string urlSuffixPseudoProperty() { result = "$UrlSuffix$" } + /** + * A taint-tracking configuration for reasoning about injection into the jQuery `$` function + * or similar, where the interpretation of the input string depends on its first character. + * + * Values are only considered tainted if they can start with the `<` character. + */ + class JQueryHtmlOrSelectorInjectionConfiguration extends TaintTracking::Configuration { + JQueryHtmlOrSelectorInjectionConfiguration() { this = "JQueryHtmlOrSelectorInjection" } + + override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) { + // Reuse any source not derived from location + source instanceof Source and + not source = DOM::locationRef() and + label.isTaint() + or + source = DOM::locationSource() and + label.isData() // Require transformation before reaching sink + or + source = DOM::locationRef().getAPropertyRead(["hash", "search"]) and + label.isData() // Require transformation before reaching sink + } + + override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) { + sink instanceof JQueryHtmlOrSelectorSink and label.isTaint() + } + + override predicate isAdditionalFlowStep( + DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel predlbl, + DataFlow::FlowLabel succlbl + ) { + exists(TaintTracking::AdditionalTaintStep step | + step.step(pred, succ) and + predlbl.isData() and + succlbl.isTaint() + ) + } + + override predicate isSanitizer(DataFlow::Node node) { + super.isSanitizer(node) + or + node instanceof Sanitizer + } + + override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) { + guard instanceof SanitizerGuard + } + + override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { + DomBasedXss::isOptionallySanitizedEdge(pred, succ) + or + // Avoid stepping from location -> location.hash, as the .hash is already treated as a source + // (with a different flow label) + exists(DataFlow::PropRead read | + read = DOM::locationRef().getAPropertyRead(["hash", "search"]) and + pred = read.getBase() and + succ = read + ) + } + } } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll index 2e2fe77d21b..2ddc0fca917 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll @@ -34,18 +34,11 @@ module UnsafeJQueryPlugin { /** * An argument that may act as a HTML fragment rather than a CSS selector, as a sink for remote unsafe jQuery plugins. */ - class AmbiguousHtmlOrSelectorArgument extends DataFlow::Node { + class AmbiguousHtmlOrSelectorArgument extends DataFlow::Node, + DomBasedXss::JQueryHtmlOrSelectorArgument { AmbiguousHtmlOrSelectorArgument() { - exists(JQuery::MethodCall call | - call.interpretsArgumentAsSelector(this) and call.interpretsArgumentAsHtml(this) - ) and - // the $-function in particular will not construct HTML for non-string values - analyze().getAType() = TTString() and // any fixed prefix makes the call unambiguous - not exists(DataFlow::Node prefix | - DomBasedXss::isPrefixOfJQueryHtmlString(this, prefix) and - prefix.mayHaveStringValue(_) - ) + not exists(getAPrefix()) } } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll index 346ff52c862..1538343f973 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll @@ -3,6 +3,7 @@ */ import javascript +private import semmle.javascript.dataflow.InferredTypes /** Provides classes and predicates shared between the XSS queries. */ module Shared { @@ -153,19 +154,9 @@ module DomBasedXss { class LibrarySink extends Sink, DataFlow::ValueNode { LibrarySink() { // call to a jQuery method that interprets its argument as HTML - exists(JQuery::MethodCall call | call.interpretsArgumentAsHtml(this) | - // either the argument is always interpreted as HTML - not call.interpretsArgumentAsSelector(this) - or - // or it doesn't start with something other than `<`, and so at least - // _may_ be interpreted as HTML - not exists(DataFlow::Node prefix, string strval | - isPrefixOfJQueryHtmlString(this, prefix) and - strval = prefix.getStringValue() and - not strval = "" and - not strval.regexpMatch("\\s*<.*") - ) and - not DOM::locationRef().flowsTo(this) + exists(JQuery::MethodCall call | + call.interpretsArgumentAsHtml(this) and + not call.interpretsArgumentAsSelector(this) // Handled by `JQuerySelectorSink` ) or // call to an Angular method that interprets its argument as HTML @@ -192,16 +183,54 @@ module DomBasedXss { * HTML by a jQuery method. */ predicate isPrefixOfJQueryHtmlString(DataFlow::Node htmlString, DataFlow::Node prefix) { - any(JQuery::MethodCall call).interpretsArgumentAsHtml(htmlString) and - prefix = htmlString + prefix = getAPrefixOfJQuerySelectorString(htmlString) + } + + /** + * Holds if `prefix` is a prefix of `htmlString`, which may be intepreted as + * HTML by a jQuery method. + */ + private DataFlow::Node getAPrefixOfJQuerySelectorString(DataFlow::Node htmlString) { + any(JQuery::MethodCall call).interpretsArgumentAsSelector(htmlString) and + result = htmlString or - exists(DataFlow::Node pred | isPrefixOfJQueryHtmlString(htmlString, pred) | - prefix = StringConcatenation::getFirstOperand(pred) + exists(DataFlow::Node pred | pred = getAPrefixOfJQuerySelectorString(htmlString) | + result = StringConcatenation::getFirstOperand(pred) or - prefix = pred.getAPredecessor() + result = pred.getAPredecessor() ) } + /** + * An argument to the jQuery `$` function or similar, which is interpreted as either a selector + * or as an HTML string depending on its first character. + */ + class JQueryHtmlOrSelectorArgument extends DataFlow::Node { + JQueryHtmlOrSelectorArgument() { + exists(JQuery::MethodCall call | + call.interpretsArgumentAsHtml(this) and + call.interpretsArgumentAsSelector(this) and + analyze().getAType() = TTString() + ) + } + + /** Gets a string that flows to the prefix of this argument. */ + string getAPrefix() { result = getAPrefixOfJQuerySelectorString(this).getStringValue() } + } + + /** + * An argument to the jQuery `$` function or similar, which may be interpreted as HTML. + * + * This is the same as `JQueryHtmlOrSelectorArgument`, excluding cases where the value + * is prefixed by something other than `<`. + */ + class JQueryHtmlOrSelectorSink extends Sink, JQueryHtmlOrSelectorArgument { + JQueryHtmlOrSelectorSink() { + // If a prefix of the string is known, it must start with '<' or be an empty string + forall(string strval | strval = getAPrefix() | strval.regexpMatch("(?s)\\s*<.*|")) + } + } + /** * An expression whose value is interpreted as HTML or CSS * and may be inserted into the DOM. @@ -350,11 +379,6 @@ module DomBasedXss { exists(PropAccess pacc | pacc = this.asExpr() | isSafeLocationProperty(pacc) or - // `$(location.hash)` is a fairly common and safe idiom - // (because `location.hash` always starts with `#`), - // so we mark `hash` as safe for the purposes of this query - pacc.getPropertyName() = "hash" - or pacc.getPropertyName() = "length" ) } diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected index ffdf900944c..1cbde553e3a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected @@ -60,17 +60,38 @@ nodes | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:7:2:40 | tainted | | jquery.js:2:17:2:33 | document.location | | jquery.js:2:17:2:33 | document.location | | jquery.js:2:17:2:40 | documen ... .search | -| jquery.js:4:5:4:11 | tainted | -| jquery.js:4:5:4:11 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | +| jquery.js:2:17:2:40 | documen ... .search | | jquery.js:7:5:7:34 | "
" | | jquery.js:7:5:7:34 | "
" | | jquery.js:7:20:7:26 | tainted | | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:28:8:34 | tainted | +| jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:13:10:20 | location | +| jquery.js:10:13:10:20 | location | +| jquery.js:10:13:10:31 | location.toString() | +| jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:38:14:52 | window.location | +| jquery.js:14:38:14:52 | window.location | +| jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:38:15:52 | window.location | +| jquery.js:15:38:15:52 | window.location | +| jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:38:16:52 | window.location | +| jquery.js:16:38:16:52 | window.location | +| jquery.js:16:38:16:63 | window. ... tring() | | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | @@ -223,9 +244,12 @@ nodes | tst3.js:10:38:10:43 | data.p | | tst3.js:10:38:10:43 | data.p | | tst.js:2:7:2:39 | target | +| tst.js:2:7:2:39 | target | | tst.js:2:16:2:32 | document.location | | tst.js:2:16:2:32 | document.location | | tst.js:2:16:2:39 | documen ... .search | +| tst.js:2:16:2:39 | documen ... .search | +| tst.js:2:16:2:39 | documen ... .search | | tst.js:5:18:5:23 | target | | tst.js:5:18:5:23 | target | | tst.js:8:18:8:126 | "" | @@ -444,6 +468,7 @@ nodes | tst.js:332:18:332:35 | params.get('name') | | tst.js:341:20:341:36 | document.location | | tst.js:341:20:341:36 | document.location | +| tst.js:343:5:343:17 | getUrl().hash | | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:348:7:348:39 | target | @@ -495,18 +520,22 @@ nodes | tst.js:416:7:416:46 | payload | | tst.js:416:17:416:31 | window.location | | tst.js:416:17:416:31 | window.location | +| tst.js:416:17:416:36 | window.location.hash | | tst.js:416:17:416:46 | window. ... bstr(1) | | tst.js:417:18:417:24 | payload | | tst.js:417:18:417:24 | payload | | tst.js:419:7:419:55 | match | | tst.js:419:15:419:29 | window.location | | tst.js:419:15:419:29 | window.location | +| tst.js:419:15:419:34 | window.location.hash | | tst.js:419:15:419:55 | window. ... (\\w+)/) | | tst.js:421:20:421:24 | match | | tst.js:421:20:421:27 | match[1] | | tst.js:421:20:421:27 | match[1] | | tst.js:424:18:424:32 | window.location | | tst.js:424:18:424:32 | window.location | +| tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:48 | window. ... it('#') | | tst.js:424:18:424:51 | window. ... '#')[1] | | tst.js:424:18:424:51 | window. ... '#')[1] | | typeahead.js:20:13:20:45 | target | @@ -572,17 +601,33 @@ edges | angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x | | angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | -| jquery.js:2:7:2:40 | tainted | jquery.js:4:5:4:11 | tainted | -| jquery.js:2:7:2:40 | tainted | jquery.js:4:5:4:11 | tainted | | jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted | | jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted | | jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search | | jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search | | jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | | jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "
" | | jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "
" | | jquery.js:8:28:8:34 | tainted | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:28:8:34 | tainted | jquery.js:8:18:8:34 | "XSS: " + tainted | +| jquery.js:10:13:10:20 | location | jquery.js:10:13:10:31 | location.toString() | +| jquery.js:10:13:10:20 | location | jquery.js:10:13:10:31 | location.toString() | +| jquery.js:10:13:10:31 | location.toString() | jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:13:10:31 | location.toString() | jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:14:38:14:52 | window.location | jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:14:38:14:52 | window.location | jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:14:38:14:57 | window.location.hash | jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:38:14:57 | window.location.hash | jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:15:38:15:52 | window.location | jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:15:38:15:52 | window.location | jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:15:38:15:59 | window. ... .search | jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:38:15:59 | window. ... .search | jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:16:38:16:52 | window.location | jquery.js:16:38:16:63 | window. ... tring() | +| jquery.js:16:38:16:52 | window.location | jquery.js:16:38:16:63 | window. ... tring() | +| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | @@ -731,6 +776,8 @@ edges | tst.js:2:16:2:32 | document.location | tst.js:2:16:2:39 | documen ... .search | | tst.js:2:16:2:32 | document.location | tst.js:2:16:2:39 | documen ... .search | | tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | +| tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | +| tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | | tst.js:8:37:8:53 | document.location | tst.js:8:37:8:58 | documen ... on.href | | tst.js:8:37:8:53 | document.location | tst.js:8:37:8:58 | documen ... on.href | | tst.js:8:37:8:58 | documen ... on.href | tst.js:8:37:8:114 | documen ... t=")+8) | @@ -916,10 +963,10 @@ edges | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | +| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:17 | getUrl().hash | +| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:17 | getUrl().hash | +| tst.js:343:5:343:17 | getUrl().hash | tst.js:343:5:343:30 | getUrl( ... ring(1) | +| tst.js:343:5:343:17 | getUrl().hash | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:348:7:348:39 | target | tst.js:349:12:349:17 | target | | tst.js:348:7:348:39 | target | tst.js:349:12:349:17 | target | | tst.js:348:16:348:32 | document.location | tst.js:348:16:348:39 | documen ... .search | @@ -964,19 +1011,22 @@ edges | tst.js:408:19:408:31 | target.taint8 | tst.js:409:18:409:30 | target.taint8 | | tst.js:416:7:416:46 | payload | tst.js:417:18:417:24 | payload | | tst.js:416:7:416:46 | payload | tst.js:417:18:417:24 | payload | -| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:46 | window. ... bstr(1) | -| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:46 | window. ... bstr(1) | +| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:36 | window.location.hash | +| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:36 | window.location.hash | +| tst.js:416:17:416:36 | window.location.hash | tst.js:416:17:416:46 | window. ... bstr(1) | | tst.js:416:17:416:46 | window. ... bstr(1) | tst.js:416:7:416:46 | payload | | tst.js:419:7:419:55 | match | tst.js:421:20:421:24 | match | -| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:55 | window. ... (\\w+)/) | -| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:55 | window. ... (\\w+)/) | +| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:34 | window.location.hash | +| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:34 | window.location.hash | +| tst.js:419:15:419:34 | window.location.hash | tst.js:419:15:419:55 | window. ... (\\w+)/) | | tst.js:419:15:419:55 | window. ... (\\w+)/) | tst.js:419:7:419:55 | match | | tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | | tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | +| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | +| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | +| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | | typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target | | typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search | | typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search | @@ -1013,9 +1063,12 @@ edges | angular2-client.ts:35:44:35:91 | this.ro ... arams.x | angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x | Cross-site scripting vulnerability due to $@. | angular2-client.ts:35:44:35:89 | this.ro ... .params | user-provided value | | angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | Cross-site scripting vulnerability due to $@. | angular2-client.ts:37:44:37:58 | this.router.url | user-provided value | | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | Cross-site scripting vulnerability due to $@. | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | user-provided value | -| jquery.js:4:5:4:11 | tainted | jquery.js:2:17:2:33 | document.location | jquery.js:4:5:4:11 | tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value | -| jquery.js:7:5:7:34 | "
" | jquery.js:2:17:2:33 | document.location | jquery.js:7:5:7:34 | "
" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value | +| jquery.js:7:5:7:34 | "
" | jquery.js:2:17:2:40 | documen ... .search | jquery.js:7:5:7:34 | "
" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value | | jquery.js:8:18:8:34 | "XSS: " + tainted | jquery.js:2:17:2:33 | document.location | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value | +| jquery.js:10:5:10:40 | "" + ... "" | jquery.js:10:13:10:20 | location | jquery.js:10:5:10:40 | "" + ... "" | Cross-site scripting vulnerability due to $@. | jquery.js:10:13:10:20 | location | user-provided value | +| jquery.js:14:19:14:58 | decodeU ... n.hash) | jquery.js:14:38:14:52 | window.location | jquery.js:14:19:14:58 | decodeU ... n.hash) | Cross-site scripting vulnerability due to $@. | jquery.js:14:38:14:52 | window.location | user-provided value | +| jquery.js:15:19:15:60 | decodeU ... search) | jquery.js:15:38:15:52 | window.location | jquery.js:15:19:15:60 | decodeU ... search) | Cross-site scripting vulnerability due to $@. | jquery.js:15:38:15:52 | window.location | user-provided value | +| jquery.js:16:19:16:64 | decodeU ... ring()) | jquery.js:16:38:16:52 | window.location | jquery.js:16:19:16:64 | decodeU ... ring()) | Cross-site scripting vulnerability due to $@. | jquery.js:16:38:16:52 | window.location | user-provided value | | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | HTML injection vulnerability due to $@. | nodemailer.js:13:50:13:66 | req.query.message | user-provided value | | optionalSanitizer.js:6:18:6:23 | target | optionalSanitizer.js:2:16:2:32 | document.location | optionalSanitizer.js:6:18:6:23 | target | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:2:16:2:32 | document.location | user-provided value | | optionalSanitizer.js:9:18:9:24 | tainted | optionalSanitizer.js:2:16:2:32 | document.location | optionalSanitizer.js:9:18:9:24 | tainted | Cross-site scripting vulnerability due to $@. | optionalSanitizer.js:2:16:2:32 | document.location | user-provided value | @@ -1051,7 +1104,7 @@ edges | tst3.js:10:38:10:43 | data.p | tst3.js:2:42:2:56 | window.location | tst3.js:10:38:10:43 | data.p | Cross-site scripting vulnerability due to $@. | tst3.js:2:42:2:56 | window.location | user-provided value | | tst.js:5:18:5:23 | target | tst.js:2:16:2:32 | document.location | tst.js:5:18:5:23 | target | Cross-site scripting vulnerability due to $@. | tst.js:2:16:2:32 | document.location | user-provided value | | tst.js:8:18:8:126 | "" | tst.js:8:37:8:53 | document.location | tst.js:8:18:8:126 | "" | Cross-site scripting vulnerability due to $@. | tst.js:8:37:8:53 | document.location | user-provided value | -| tst.js:12:5:12:42 | '
' | tst.js:2:16:2:32 | document.location | tst.js:12:5:12:42 | '
' | Cross-site scripting vulnerability due to $@. | tst.js:2:16:2:32 | document.location | user-provided value | +| tst.js:12:5:12:42 | '
' | tst.js:2:16:2:39 | documen ... .search | tst.js:12:5:12:42 | '
' | Cross-site scripting vulnerability due to $@. | tst.js:2:16:2:39 | documen ... .search | user-provided value | | tst.js:18:18:18:35 | params.get('name') | tst.js:17:25:17:41 | document.location | tst.js:18:18:18:35 | params.get('name') | Cross-site scripting vulnerability due to $@. | tst.js:17:25:17:41 | document.location | user-provided value | | tst.js:21:18:21:41 | searchP ... 'name') | tst.js:2:16:2:32 | document.location | tst.js:21:18:21:41 | searchP ... 'name') | Cross-site scripting vulnerability due to $@. | tst.js:2:16:2:32 | document.location | user-provided value | | tst.js:26:18:26:23 | target | tst.js:28:5:28:21 | document.location | tst.js:26:18:26:23 | target | Cross-site scripting vulnerability due to $@. | tst.js:28:5:28:21 | document.location | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected index ec54e329e69..47a8a8737dc 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected @@ -60,17 +60,38 @@ nodes | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:7:2:40 | tainted | | jquery.js:2:17:2:33 | document.location | | jquery.js:2:17:2:33 | document.location | | jquery.js:2:17:2:40 | documen ... .search | -| jquery.js:4:5:4:11 | tainted | -| jquery.js:4:5:4:11 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | +| jquery.js:2:17:2:40 | documen ... .search | | jquery.js:7:5:7:34 | "
" | | jquery.js:7:5:7:34 | "
" | | jquery.js:7:20:7:26 | tainted | | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:28:8:34 | tainted | +| jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:13:10:20 | location | +| jquery.js:10:13:10:20 | location | +| jquery.js:10:13:10:31 | location.toString() | +| jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:38:14:52 | window.location | +| jquery.js:14:38:14:52 | window.location | +| jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:38:15:52 | window.location | +| jquery.js:15:38:15:52 | window.location | +| jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:38:16:52 | window.location | +| jquery.js:16:38:16:52 | window.location | +| jquery.js:16:38:16:63 | window. ... tring() | | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | @@ -223,9 +244,12 @@ nodes | tst3.js:10:38:10:43 | data.p | | tst3.js:10:38:10:43 | data.p | | tst.js:2:7:2:39 | target | +| tst.js:2:7:2:39 | target | | tst.js:2:16:2:32 | document.location | | tst.js:2:16:2:32 | document.location | | tst.js:2:16:2:39 | documen ... .search | +| tst.js:2:16:2:39 | documen ... .search | +| tst.js:2:16:2:39 | documen ... .search | | tst.js:5:18:5:23 | target | | tst.js:5:18:5:23 | target | | tst.js:8:18:8:126 | "" | @@ -444,6 +468,7 @@ nodes | tst.js:332:18:332:35 | params.get('name') | | tst.js:341:20:341:36 | document.location | | tst.js:341:20:341:36 | document.location | +| tst.js:343:5:343:17 | getUrl().hash | | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:348:7:348:39 | target | @@ -495,18 +520,22 @@ nodes | tst.js:416:7:416:46 | payload | | tst.js:416:17:416:31 | window.location | | tst.js:416:17:416:31 | window.location | +| tst.js:416:17:416:36 | window.location.hash | | tst.js:416:17:416:46 | window. ... bstr(1) | | tst.js:417:18:417:24 | payload | | tst.js:417:18:417:24 | payload | | tst.js:419:7:419:55 | match | | tst.js:419:15:419:29 | window.location | | tst.js:419:15:419:29 | window.location | +| tst.js:419:15:419:34 | window.location.hash | | tst.js:419:15:419:55 | window. ... (\\w+)/) | | tst.js:421:20:421:24 | match | | tst.js:421:20:421:27 | match[1] | | tst.js:421:20:421:27 | match[1] | | tst.js:424:18:424:32 | window.location | | tst.js:424:18:424:32 | window.location | +| tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:48 | window. ... it('#') | | tst.js:424:18:424:51 | window. ... '#')[1] | | tst.js:424:18:424:51 | window. ... '#')[1] | | typeahead.js:9:28:9:30 | loc | @@ -576,17 +605,33 @@ edges | angular2-client.ts:35:44:35:89 | this.ro ... .params | angular2-client.ts:35:44:35:91 | this.ro ... arams.x | | angular2-client.ts:37:44:37:58 | this.router.url | angular2-client.ts:37:44:37:58 | this.router.url | | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | angular2-client.ts:41:44:41:76 | routeSn ... ('foo') | -| jquery.js:2:7:2:40 | tainted | jquery.js:4:5:4:11 | tainted | -| jquery.js:2:7:2:40 | tainted | jquery.js:4:5:4:11 | tainted | | jquery.js:2:7:2:40 | tainted | jquery.js:7:20:7:26 | tainted | | jquery.js:2:7:2:40 | tainted | jquery.js:8:28:8:34 | tainted | | jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search | | jquery.js:2:17:2:33 | document.location | jquery.js:2:17:2:40 | documen ... .search | | jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | +| jquery.js:2:17:2:40 | documen ... .search | jquery.js:2:7:2:40 | tainted | | jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "
" | | jquery.js:7:20:7:26 | tainted | jquery.js:7:5:7:34 | "
" | | jquery.js:8:28:8:34 | tainted | jquery.js:8:18:8:34 | "XSS: " + tainted | | jquery.js:8:28:8:34 | tainted | jquery.js:8:18:8:34 | "XSS: " + tainted | +| jquery.js:10:13:10:20 | location | jquery.js:10:13:10:31 | location.toString() | +| jquery.js:10:13:10:20 | location | jquery.js:10:13:10:31 | location.toString() | +| jquery.js:10:13:10:31 | location.toString() | jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:10:13:10:31 | location.toString() | jquery.js:10:5:10:40 | "" + ... "" | +| jquery.js:14:38:14:52 | window.location | jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:14:38:14:52 | window.location | jquery.js:14:38:14:57 | window.location.hash | +| jquery.js:14:38:14:57 | window.location.hash | jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:14:38:14:57 | window.location.hash | jquery.js:14:19:14:58 | decodeU ... n.hash) | +| jquery.js:15:38:15:52 | window.location | jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:15:38:15:52 | window.location | jquery.js:15:38:15:59 | window. ... .search | +| jquery.js:15:38:15:59 | window. ... .search | jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:15:38:15:59 | window. ... .search | jquery.js:15:19:15:60 | decodeU ... search) | +| jquery.js:16:38:16:52 | window.location | jquery.js:16:38:16:63 | window. ... tring() | +| jquery.js:16:38:16:52 | window.location | jquery.js:16:38:16:63 | window. ... tring() | +| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) | +| jquery.js:16:38:16:63 | window. ... tring() | jquery.js:16:19:16:64 | decodeU ... ring()) | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | | nodemailer.js:13:50:13:66 | req.query.message | nodemailer.js:13:11:13:69 | `Hi, yo ... sage}.` | @@ -735,6 +780,8 @@ edges | tst.js:2:16:2:32 | document.location | tst.js:2:16:2:39 | documen ... .search | | tst.js:2:16:2:32 | document.location | tst.js:2:16:2:39 | documen ... .search | | tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | +| tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | +| tst.js:2:16:2:39 | documen ... .search | tst.js:2:7:2:39 | target | | tst.js:8:37:8:53 | document.location | tst.js:8:37:8:58 | documen ... on.href | | tst.js:8:37:8:53 | document.location | tst.js:8:37:8:58 | documen ... on.href | | tst.js:8:37:8:58 | documen ... on.href | tst.js:8:37:8:114 | documen ... t=")+8) | @@ -920,10 +967,10 @@ edges | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | | tst.js:327:18:327:34 | document.location | tst.js:332:18:332:35 | params.get('name') | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | -| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:30 | getUrl( ... ring(1) | +| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:17 | getUrl().hash | +| tst.js:341:20:341:36 | document.location | tst.js:343:5:343:17 | getUrl().hash | +| tst.js:343:5:343:17 | getUrl().hash | tst.js:343:5:343:30 | getUrl( ... ring(1) | +| tst.js:343:5:343:17 | getUrl().hash | tst.js:343:5:343:30 | getUrl( ... ring(1) | | tst.js:348:7:348:39 | target | tst.js:349:12:349:17 | target | | tst.js:348:7:348:39 | target | tst.js:349:12:349:17 | target | | tst.js:348:16:348:32 | document.location | tst.js:348:16:348:39 | documen ... .search | @@ -968,19 +1015,22 @@ edges | tst.js:408:19:408:31 | target.taint8 | tst.js:409:18:409:30 | target.taint8 | | tst.js:416:7:416:46 | payload | tst.js:417:18:417:24 | payload | | tst.js:416:7:416:46 | payload | tst.js:417:18:417:24 | payload | -| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:46 | window. ... bstr(1) | -| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:46 | window. ... bstr(1) | +| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:36 | window.location.hash | +| tst.js:416:17:416:31 | window.location | tst.js:416:17:416:36 | window.location.hash | +| tst.js:416:17:416:36 | window.location.hash | tst.js:416:17:416:46 | window. ... bstr(1) | | tst.js:416:17:416:46 | window. ... bstr(1) | tst.js:416:7:416:46 | payload | | tst.js:419:7:419:55 | match | tst.js:421:20:421:24 | match | -| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:55 | window. ... (\\w+)/) | -| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:55 | window. ... (\\w+)/) | +| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:34 | window.location.hash | +| tst.js:419:15:419:29 | window.location | tst.js:419:15:419:34 | window.location.hash | +| tst.js:419:15:419:34 | window.location.hash | tst.js:419:15:419:55 | window. ... (\\w+)/) | | tst.js:419:15:419:55 | window. ... (\\w+)/) | tst.js:419:7:419:55 | match | | tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | | tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | -| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | +| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:32 | window.location | tst.js:424:18:424:37 | window.location.hash | +| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | +| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | +| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | | typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc | | typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc | | typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.ql b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.ql index 96dee32082b..b532f0b8a75 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.ql +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.ql @@ -16,7 +16,7 @@ import semmle.javascript.security.dataflow.DomBasedXss::DomBasedXss import DataFlow::PathGraph import semmle.javascript.heuristics.AdditionalSources -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink where cfg.hasFlowPath(source, sink) and source.getNode() instanceof HeuristicSource select sink.getNode(), source, sink, sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.", source.getNode(), diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery.js index 46ca34c8c12..928648ced0f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery.js @@ -1,10 +1,17 @@ function test() { var tainted = document.location.search - $(tainted); // NOT OK + $(tainted); // OK - location.search starts with '?' $("body", tainted); // OK $("." + tainted); // OK $("
"); // NOT OK $("body").html("XSS: " + tainted); // NOT OK - $(window.location.hash); // OK + $(window.location.hash); // OK - location.hash starts with '#' + $("" + location.toString() + ""); // NOT OK + + // Not related to jQuery, but the handling of $() should not affect this sink + let elm = document.getElementById('x'); + elm.innerHTML = decodeURIComponent(window.location.hash); // NOT OK + elm.innerHTML = decodeURIComponent(window.location.search); // NOT OK + elm.innerHTML = decodeURIComponent(window.location.toString()); // NOT OK }