From 5f05232e023536a57e5ec851b4e3608ce5ee58a1 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 4 Oct 2023 21:31:50 +0200 Subject: [PATCH] JS: Port StoredXss --- .../dataflow/StoredXssCustomizations.qll | 15 +++ .../security/dataflow/StoredXssQuery.qll | 25 ++++- .../ql/src/Security/CWE-079/StoredXss.ql | 6 +- .../CWE-079/StoredXss/StoredXss.expected | 95 ++++++++++--------- 4 files changed, 90 insertions(+), 51 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssCustomizations.qll index 16fe8e44a9c..b0de349a53d 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssCustomizations.qll @@ -21,6 +21,21 @@ module StoredXss { /** A sanitizer for stored XSS vulnerabilities. */ abstract class Sanitizer extends Shared::Sanitizer { } + /** + * A barrier guard for stored XSS. + */ + abstract class BarrierGuard extends DataFlow::Node { + /** + * Holds if this node acts as a barrier for data flow, blocking further flow from `e` if `this` evaluates to `outcome`. + */ + predicate blocksExpr(boolean outcome, Expr e) { none() } + } + + /** A subclass of `BarrierGuard` that is used for backward compatibility with the old data flow library. */ + abstract class BarrierGuardLegacy extends BarrierGuard, TaintTracking::SanitizerGuardNode { + override predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) } + } + /** An arbitrary XSS sink, considered as a flow sink for stored XSS. */ private class AnySink extends Sink { AnySink() { this instanceof Shared::Sink } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssQuery.qll index cc2f3947186..b40b610b71e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/StoredXssQuery.qll @@ -8,9 +8,25 @@ import StoredXssCustomizations::StoredXss private import Xss::Shared as Shared /** - * A taint-tracking configuration for reasoning about XSS. + * A taint-tracking configuration for reasoning about stored XSS. */ -class Configuration extends TaintTracking::Configuration { +module StoredXssConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof Source } + + predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } +} + +/** + * Taint-tracking for reasoning about stored XSS. + */ +module StoredXssFlow = TaintTracking::Global; + +/** + * DEPRECATED. Use the `StoredXssFlow` module instead. + */ +deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "StoredXss" } override predicate isSource(DataFlow::Node source) { source instanceof Source } @@ -28,11 +44,10 @@ class Configuration extends TaintTracking::Configuration { } } -private class QuoteGuard extends TaintTracking::SanitizerGuardNode, Shared::QuoteGuard { +private class QuoteGuard extends Shared::QuoteGuard { QuoteGuard() { this = this } } -private class ContainsHtmlGuard extends TaintTracking::SanitizerGuardNode, Shared::ContainsHtmlGuard -{ +private class ContainsHtmlGuard extends Shared::ContainsHtmlGuard { ContainsHtmlGuard() { this = this } } diff --git a/javascript/ql/src/Security/CWE-079/StoredXss.ql b/javascript/ql/src/Security/CWE-079/StoredXss.ql index d5f28b28e55..5aabb1cd115 100644 --- a/javascript/ql/src/Security/CWE-079/StoredXss.ql +++ b/javascript/ql/src/Security/CWE-079/StoredXss.ql @@ -14,9 +14,9 @@ import javascript import semmle.javascript.security.dataflow.StoredXssQuery -import DataFlow::PathGraph +import StoredXssFlow::PathGraph -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from StoredXssFlow::PathNode source, StoredXssFlow::PathNode sink +where StoredXssFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Stored cross-site scripting vulnerability due to $@.", source.getNode(), "stored value" diff --git a/javascript/ql/test/query-tests/Security/CWE-079/StoredXss/StoredXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/StoredXss/StoredXss.expected index d6142c980b6..53f02ae19f2 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/StoredXss/StoredXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/StoredXss/StoredXss.expected @@ -1,55 +1,64 @@ -nodes -| xss-through-filenames.js:7:43:7:48 | files1 | -| xss-through-filenames.js:7:43:7:48 | files1 | -| xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | -| xss-through-filenames.js:26:19:26:24 | files1 | -| xss-through-filenames.js:26:19:26:24 | files1 | -| xss-through-filenames.js:29:13:29:23 | files2 | -| xss-through-filenames.js:29:22:29:23 | [] | -| xss-through-filenames.js:30:9:30:14 | files1 | -| xss-through-filenames.js:30:34:30:37 | file | -| xss-through-filenames.js:31:25:31:28 | file | -| xss-through-filenames.js:33:19:33:24 | files2 | -| xss-through-filenames.js:33:19:33:24 | files2 | -| xss-through-filenames.js:35:13:35:35 | files3 | -| xss-through-filenames.js:35:22:35:35 | format(files2) | -| xss-through-filenames.js:35:29:35:34 | files2 | -| xss-through-filenames.js:37:19:37:24 | files3 | -| xss-through-filenames.js:37:19:37:24 | files3 | -| xss-through-torrent.js:6:6:6:24 | name | -| xss-through-torrent.js:6:13:6:24 | torrent.name | -| xss-through-torrent.js:6:13:6:24 | torrent.name | -| xss-through-torrent.js:7:11:7:14 | name | -| xss-through-torrent.js:7:11:7:14 | name | edges | xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | +| xss-through-filenames.js:17:21:17:26 | files2 | xss-through-filenames.js:19:9:19:14 | files2 | +| xss-through-filenames.js:17:21:17:26 | files2 [ArrayElement] | xss-through-filenames.js:19:9:19:14 | files2 [ArrayElement] | +| xss-through-filenames.js:19:9:19:14 | files2 | xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | +| xss-through-filenames.js:19:9:19:14 | files2 | xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | +| xss-through-filenames.js:19:9:19:14 | files2 [ArrayElement] | xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | +| xss-through-filenames.js:19:9:19:14 | files2 [ArrayElement] | xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | xss-through-filenames.js:22:16:22:21 | files3 | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | xss-through-filenames.js:22:16:22:21 | files3 | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | xss-through-filenames.js:22:16:22:21 | files3 | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | xss-through-filenames.js:22:16:22:21 | files3 | +| xss-through-filenames.js:22:16:22:21 | files3 | xss-through-filenames.js:22:16:22:30 | files3.join('') | +| xss-through-filenames.js:22:16:22:21 | files3 | xss-through-filenames.js:22:16:22:30 | files3.join('') | | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:30:9:30:14 | files1 | -| xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:30:9:30:14 | files1 | -| xss-through-filenames.js:29:13:29:23 | files2 | xss-through-filenames.js:33:19:33:24 | files2 | -| xss-through-filenames.js:29:13:29:23 | files2 | xss-through-filenames.js:33:19:33:24 | files2 | -| xss-through-filenames.js:29:13:29:23 | files2 | xss-through-filenames.js:35:29:35:34 | files2 | -| xss-through-filenames.js:29:22:29:23 | [] | xss-through-filenames.js:29:13:29:23 | files2 | -| xss-through-filenames.js:30:9:30:14 | files1 | xss-through-filenames.js:30:34:30:37 | file | -| xss-through-filenames.js:30:34:30:37 | file | xss-through-filenames.js:31:25:31:28 | file | -| xss-through-filenames.js:31:25:31:28 | file | xss-through-filenames.js:29:22:29:23 | [] | -| xss-through-filenames.js:35:13:35:35 | files3 | xss-through-filenames.js:37:19:37:24 | files3 | +| xss-through-filenames.js:30:9:30:14 | files1 | xss-through-filenames.js:33:19:33:24 | files2 | +| xss-through-filenames.js:30:9:30:14 | files1 | xss-through-filenames.js:33:19:33:24 | files2 | +| xss-through-filenames.js:30:9:30:14 | files1 | xss-through-filenames.js:33:19:33:24 | files2 [ArrayElement] | +| xss-through-filenames.js:33:19:33:24 | files2 | xss-through-filenames.js:35:29:35:34 | files2 | +| xss-through-filenames.js:33:19:33:24 | files2 [ArrayElement] | xss-through-filenames.js:35:29:35:34 | files2 [ArrayElement] | | xss-through-filenames.js:35:13:35:35 | files3 | xss-through-filenames.js:37:19:37:24 | files3 | | xss-through-filenames.js:35:22:35:35 | format(files2) | xss-through-filenames.js:35:13:35:35 | files3 | +| xss-through-filenames.js:35:29:35:34 | files2 | xss-through-filenames.js:17:21:17:26 | files2 | | xss-through-filenames.js:35:29:35:34 | files2 | xss-through-filenames.js:35:22:35:35 | format(files2) | -| xss-through-torrent.js:6:6:6:24 | name | xss-through-torrent.js:7:11:7:14 | name | +| xss-through-filenames.js:35:29:35:34 | files2 [ArrayElement] | xss-through-filenames.js:17:21:17:26 | files2 [ArrayElement] | +| xss-through-filenames.js:35:29:35:34 | files2 [ArrayElement] | xss-through-filenames.js:35:22:35:35 | format(files2) | | xss-through-torrent.js:6:6:6:24 | name | xss-through-torrent.js:7:11:7:14 | name | | xss-through-torrent.js:6:13:6:24 | torrent.name | xss-through-torrent.js:6:6:6:24 | name | -| xss-through-torrent.js:6:13:6:24 | torrent.name | xss-through-torrent.js:6:6:6:24 | name | +nodes +| xss-through-filenames.js:7:43:7:48 | files1 | semmle.label | files1 | +| xss-through-filenames.js:8:18:8:23 | files1 | semmle.label | files1 | +| xss-through-filenames.js:17:21:17:26 | files2 | semmle.label | files2 | +| xss-through-filenames.js:17:21:17:26 | files2 [ArrayElement] | semmle.label | files2 [ArrayElement] | +| xss-through-filenames.js:19:9:19:14 | files2 | semmle.label | files2 | +| xss-through-filenames.js:19:9:19:14 | files2 [ArrayElement] | semmle.label | files2 [ArrayElement] | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | semmle.label | files2.sort(sort) | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) | semmle.label | files2.sort(sort) | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | semmle.label | files2.sort(sort) [ArrayElement] | +| xss-through-filenames.js:19:9:19:25 | files2.sort(sort) [ArrayElement] | semmle.label | files2.sort(sort) [ArrayElement] | +| xss-through-filenames.js:22:16:22:21 | files3 | semmle.label | files3 | +| xss-through-filenames.js:22:16:22:21 | files3 | semmle.label | files3 | +| xss-through-filenames.js:22:16:22:30 | files3.join('') | semmle.label | files3.join('') | +| xss-through-filenames.js:22:16:22:30 | files3.join('') | semmle.label | files3.join('') | +| xss-through-filenames.js:25:43:25:48 | files1 | semmle.label | files1 | +| xss-through-filenames.js:26:19:26:24 | files1 | semmle.label | files1 | +| xss-through-filenames.js:30:9:30:14 | files1 | semmle.label | files1 | +| xss-through-filenames.js:33:19:33:24 | files2 | semmle.label | files2 | +| xss-through-filenames.js:33:19:33:24 | files2 | semmle.label | files2 | +| xss-through-filenames.js:33:19:33:24 | files2 [ArrayElement] | semmle.label | files2 [ArrayElement] | +| xss-through-filenames.js:35:13:35:35 | files3 | semmle.label | files3 | +| xss-through-filenames.js:35:22:35:35 | format(files2) | semmle.label | format(files2) | +| xss-through-filenames.js:35:29:35:34 | files2 | semmle.label | files2 | +| xss-through-filenames.js:35:29:35:34 | files2 [ArrayElement] | semmle.label | files2 [ArrayElement] | +| xss-through-filenames.js:37:19:37:24 | files3 | semmle.label | files3 | +| xss-through-torrent.js:6:6:6:24 | name | semmle.label | name | +| xss-through-torrent.js:6:13:6:24 | torrent.name | semmle.label | torrent.name | +| xss-through-torrent.js:7:11:7:14 | name | semmle.label | name | +subpaths +| xss-through-filenames.js:35:29:35:34 | files2 | xss-through-filenames.js:17:21:17:26 | files2 | xss-through-filenames.js:22:16:22:30 | files3.join('') | xss-through-filenames.js:35:22:35:35 | format(files2) | +| xss-through-filenames.js:35:29:35:34 | files2 [ArrayElement] | xss-through-filenames.js:17:21:17:26 | files2 [ArrayElement] | xss-through-filenames.js:22:16:22:30 | files3.join('') | xss-through-filenames.js:35:22:35:35 | format(files2) | #select | xss-through-filenames.js:8:18:8:23 | files1 | xss-through-filenames.js:7:43:7:48 | files1 | xss-through-filenames.js:8:18:8:23 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:7:43:7:48 | files1 | stored value | | xss-through-filenames.js:26:19:26:24 | files1 | xss-through-filenames.js:25:43:25:48 | files1 | xss-through-filenames.js:26:19:26:24 | files1 | Stored cross-site scripting vulnerability due to $@. | xss-through-filenames.js:25:43:25:48 | files1 | stored value |