From e86a3b5e577fc603c5cf1dd171c191b7e692e373 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 26 Apr 2021 10:21:25 +0200 Subject: [PATCH] add js/html-constructed-from-input query --- .../CWE-079/UnsafeHtmlConstruction.ql | 22 +++ .../semmle/javascript/frameworks/Markdown.qll | 3 + .../dataflow/UnsafeHtmlConstruction.qll | 32 ++++ .../UnsafeHtmlConstructionCustomizations.qll | 168 ++++++++++++++++++ 4 files changed, 225 insertions(+) create mode 100644 javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.ql create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll create mode 100644 javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll diff --git a/javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.ql b/javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.ql new file mode 100644 index 00000000000..464f4951307 --- /dev/null +++ b/javascript/ql/src/Security/CWE-079/UnsafeHtmlConstruction.ql @@ -0,0 +1,22 @@ +/** + * @name Unsafe HTML constructed from library input + * @description Using externally controlled strings to construct HTML might allow a malicious + * user to perform an cross-site scripting attack. + * @kind path-problem + * @problem.severity error + * @precision high + * @id js/html-constructed-from-input + * @tags security + * external/cwe/cwe-079 + * external/cwe/cwe-116 + */ + +import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.UnsafeHtmlConstruction::UnsafeHtmlConstruction + +from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode +where cfg.hasFlowPath(source, sink) and sink.getNode() = sinkNode +select sinkNode, source, sink, "$@ based on $@ might later cause $@.", sinkNode, + sinkNode.describe(), source.getNode(), "library input", sinkNode.getSink(), + sinkNode.getVulnerabilityKind().toLowerCase() diff --git a/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll b/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll index 788e28e2e9e..c98243e4fbb 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Markdown.qll @@ -13,6 +13,9 @@ module Markdown { * A taint-step that parses a markdown document, but preserves taint.import */ class MarkdownStep extends Unit { + /** + * Holds if there is a taint-step from `pred` to `succ` for a taint-preserving markdown parser. + */ abstract predicate step(DataFlow::Node pred, DataFlow::Node succ); } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll new file mode 100644 index 00000000000..3e2bf53bde9 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstruction.qll @@ -0,0 +1,32 @@ +/** + * Provides a taint-tracking configuration for reasoning about + * unsafe HTML constructed from library input vulnerabilities. + */ + +import javascript + +/** + * Classes and predicates for the unsafe HTML constructed from library input query. + */ +module UnsafeHtmlConstruction { + private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss + private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin + import UnsafeHtmlConstructionCustomizations::UnsafeHtmlConstruction + + /** + * A taint-tracking configuration for reasoning about unsafe HTML constructed from library input vulnerabilities. + */ + class Configration extends TaintTracking::Configuration { + Configration() { this = "UnsafeHtmlConstruction" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + override predicate isSanitizer(DataFlow::Node node) { super.isSanitizer(node) } + + override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) { + DomBasedXss::isOptionallySanitizedEdge(pred, succ) + } + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll new file mode 100644 index 00000000000..b72df1b7421 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll @@ -0,0 +1,168 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * unsafe HTML constructed from library input, as well as extension points + * for adding your own. + */ + +import javascript + +/** + * Module containing sources, sinks, and sanitizers for unsafe HTML constructed from library input. + */ +module UnsafeHtmlConstruction { + private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss + private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin + private import semmle.javascript.PackageExports as Exports + + /** + * A source for unsafe HTML constructed from library input. + */ + abstract class Source extends DataFlow::Node { } + + /** + * A parameter of an exported function, seen as a source for usnafe HTML constructed from input. + */ + class ExternalInputSource extends Source, DataFlow::ParameterNode { + ExternalInputSource() { + this = Exports::getALibraryInputParameter() and + not this = JQuery::dollarSource() + } + } + + /** + * A sink for unsafe HTML constructed from library input. + * This sink somehow transforms its input into a value that can cause XSS if it ends up in a XSS sink. + */ + abstract class Sink extends DataFlow::Node { + /** + * Gets the kind of vulnerability to report in the alert message. + * + * Defaults to `Cross-site scripting`, but may be overriden for sinks + * that do not allow script injection, but injection of other undesirable HTML elements. + */ + abstract string getVulnerabilityKind(); + + /** + * Gets the XSS sink that this transformed input ends up in. + */ + abstract DataFlow::Node getSink(); + + /** + * Gets a string describing the transformation that this sink represents. + */ + abstract string describe(); + } + + /** + * A sink for `js/html-constructed-from-input` that constructs some HTML where + * that HTML is later used in `xssSink`. + */ + abstract class XssSink extends Sink { + DomBasedXss::Sink xssSink; + + final override string getVulnerabilityKind() { result = xssSink.getVulnerabilityKind() } + + final override DomBasedXss::Sink getSink() { result = xssSink } + } + + /** + * Gets a dataflow node that flows to `sink` tracked by `t`. + */ + private DataFlow::Node isUsedInXssSink(DataFlow::TypeBackTracker t, DomBasedXss::Sink sink) { + t.start() and + result = sink + or + exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, isUsedInXssSink(t2, sink))) + or + exists(DataFlow::TypeBackTracker t2 | + t.continue() = t2 and + domBasedTaintStep(result, isUsedInXssSink(t2, sink)) + ) + } + + /** + * Gets a dataflow node that flows to `sink`. + */ + DataFlow::Node isUsedInXssSink(DomBasedXss::Sink sink) { + result = isUsedInXssSink(DataFlow::TypeBackTracker::end(), sink) + } + + /** + * Holds if there is a taint step from `pred` to `succ` for DOM strings/nodes. + * These steps are mostly relevant for DOM nodes that are created by an XML parser. + */ + predicate domBasedTaintStep(DataFlow::Node pred, DataFlow::SourceNode succ) { + // node.appendChild(newChild) and similar + exists(DataFlow::MethodCallNode call | + call.getMethodName() = ["insertBefore", "replaceChild", "appendChild"] + | + pred = call.getArgument(0) and + succ = [call.getReceiver().getALocalSource(), call] + ) + or + // element.{prepend,append}(node) and similar + exists(DataFlow::MethodCallNode call | + call.getMethodName() = ["prepend", "append", "replaceWith", "replaceChildren"] + | + pred = call.getAnArgument() and + succ = call.getReceiver().getALocalSource() + ) + or + // node.insertAdjacentElement("location", newChild) + exists(DataFlow::MethodCallNode call | call.getMethodName() = "insertAdjacentElement" | + pred = call.getArgument(1) and + succ = call.getReceiver().getALocalSource() + ) + or + // clone = node.cloneNode() + exists(DataFlow::MethodCallNode cloneNode | cloneNode.getMethodName() = "cloneNode" | + pred = cloneNode.getReceiver() and + succ = cloneNode + ) + or + // var succ = pred.documentElement; + // documentElement is the root element of the document, and childNodes is the list of all children + exists(DataFlow::PropRead read | read.getPropertyName() = ["documentElement", "childNodes"] | + pred = read.getBase() and + succ = read + ) + } + + /** + * A string-concatenation of HTML, where the result is used as an XSS sink. + */ + class HTMLConcatenationSink extends XssSink, StringOps::HtmlConcatenationLeaf { + HTMLConcatenationSink() { isUsedInXssSink(xssSink) = this.getRoot() } + + override string describe() { result = "HTML construction" } + } + + /** + * A string parsed as XML, which is later used in an XSS sink. + */ + class XMLParsedSink extends XssSink { + XML::ParserInvocation parser; + + XMLParsedSink() { + this.asExpr() = parser.getSourceArgument() and + isUsedInXssSink(xssSink) = parser.getAResult() + } + + override string describe() { result = "XML parsing" } + } + + /** + * A string rendered as markdown, where the rendering preserves HTML. + */ + class MarkdownSink extends XssSink { + MarkdownSink() { + exists(DataFlow::Node pred, DataFlow::Node succ, Markdown::MarkdownStep step | + step.step(pred, succ) and + this = pred and + succ = isUsedInXssSink(xssSink) + ) + } + + override string describe() { result = "Markdown rendering" } + } +}