convert the DOM model to use DataFlow nodes

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll

@@ -179,7 +179,7 @@ module ClientSideUrlRedirect {
       )
       or
       // e.g. node.setAttribute("href", sink)
-      any(DomMethodCallExpr call).interpretsArgumentsAsUrl(this.asExpr())
+      any(DomMethodCallNode call).interpretsArgumentsAsUrl(this)
     }
 
     override predicate isXssSink() { any() }
@@ -191,9 +191,9 @@ module ClientSideUrlRedirect {
    */
   class AttributeWriteUrlSink extends ScriptUrlSink, DataFlow::ValueNode {
     AttributeWriteUrlSink() {
-      exists(DomPropWriteNode pw |
+      exists(DomPropertyWrite pw |
         pw.interpretsValueAsJavaScriptUrl() and
-        this = DataFlow::valueNode(pw.getRhs())
+        this = pw.getRhs()
       )
     }
 

javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll

@@ -21,14 +21,28 @@ class DomGlobalVariable extends GlobalVariable {
 /** DEPRECATED: Alias for DomGlobalVariable */
 deprecated class DOMGlobalVariable = DomGlobalVariable;
 
-/** Holds if `e` could hold a value that comes from the DOM. */
-predicate isDomValue(Expr e) { DOM::domValueRef().flowsToExpr(e) }
+/**
+ * DEPRECATED: Use `isDomNode` instead.
+ * Holds if `e` could hold a value that comes from the DOM.
+ */
+deprecated predicate isDomValue(Expr e) { isDomNode(e.flow()) }
+
+/**
+ * Holds if `e` could hold a value that comes from the DOM.
+ */
+predicate isDomNode(DataFlow::Node e) { DOM::domValueRef().flowsTo(e) }
+
+/**
+ * DEPRECATED: Use `isLocationNode` instead.
+ * Holds if `e` could refer to the `location` property of a DOM node.
+ */
+deprecated predicate isLocation(Expr e) { isLocationNode(e.flow()) }
 
 /** Holds if `e` could refer to the `location` property of a DOM node. */
-predicate isLocation(Expr e) {
-  e = DOM::domValueRef().getAPropertyReference("location").asExpr()
+predicate isLocationNode(DataFlow::Node e) {
+  e = DOM::domValueRef().getAPropertyReference("location")
   or
-  e.accessesGlobal("location")
+  e = DataFlow::globalVarRef("location")
 }
 
 /**
@@ -53,15 +67,52 @@ deprecated predicate isDocumentUrl(Expr e) { e.flow() = DOM::locationSource() }
 deprecated predicate isDocumentURL = isDocumentUrl/1;
 
 /**
+ * DEPRECATED. In most cases, a sanitizer based on this predicate can be removed, as
+ * taint tracking no longer step through the properties of the location object by default.
+ *
+ * Holds if `pacc` accesses a part of `document.location` that is
+ * not considered user-controlled, that is, anything except
+ * `href`, `hash` and `search`.
+ */
+deprecated predicate isSafeLocationProperty(PropAccess pacc) {
+  exists(string prop | pacc = DOM::locationRef().getAPropertyRead(prop).asExpr() |
+    prop != "href" and prop != "hash" and prop != "search"
+  )
+}
+
+/**
+ * DEPRECATED: Use `DomMethodCallNode` instead.
  * A call to a DOM method.
  */
-class DomMethodCallExpr extends MethodCallExpr {
-  DomMethodCallExpr() { isDomValue(this.getReceiver()) }
+deprecated class DomMethodCallExpr extends MethodCallExpr {
+  DomMethodCallNode node;
+
+  DomMethodCallExpr() { this.flow() = node }
+
+  /** Holds if `arg` is an argument that is interpreted as HTML. */
+  deprecated predicate interpretsArgumentsAsHtml(Expr arg) {
+    node.interpretsArgumentsAsHtml(arg.flow())
+  }
+
+  /** Holds if `arg` is an argument that is used as an URL. */
+  deprecated predicate interpretsArgumentsAsURL(Expr arg) {
+    node.interpretsArgumentsAsURL(arg.flow())
+  }
+
+  /** DEPRECATED: Alias for interpretsArgumentsAsHtml */
+  deprecated predicate interpretsArgumentsAsHTML(Expr arg) { this.interpretsArgumentsAsHtml(arg) }
+}
+
+/**
+ * A call to a DOM method.
+ */
+class DomMethodCallNode extends DataFlow::MethodCallNode {
+  DomMethodCallNode() { isDomNode(this.getReceiver()) }
 
   /**
    * Holds if `arg` is an argument that is interpreted as HTML.
    */
-  predicate interpretsArgumentsAsHtml(Expr arg) {
+  predicate interpretsArgumentsAsHtml(DataFlow::Node arg) {
     exists(int argPos, string name |
       arg = this.getArgument(argPos) and
       name = this.getMethodName()
@@ -86,7 +137,7 @@ class DomMethodCallExpr extends MethodCallExpr {
   /**
    * Holds if `arg` is an argument that is used as an URL.
    */
-  predicate interpretsArgumentsAsUrl(Expr arg) {
+  predicate interpretsArgumentsAsUrl(DataFlow::Node arg) {
     exists(int argPos, string name |
       arg = this.getArgument(argPos) and
       name = this.getMethodName()
@@ -104,40 +155,60 @@ class DomMethodCallExpr extends MethodCallExpr {
   }
 
   /** DEPRECATED: Alias for interpretsArgumentsAsUrl */
-  deprecated predicate interpretsArgumentsAsURL(Expr arg) { this.interpretsArgumentsAsUrl(arg) }
+  deprecated predicate interpretsArgumentsAsURL(DataFlow::Node arg) { this.interpretsArgumentsAsUrl(arg) }
 
   /** DEPRECATED: Alias for interpretsArgumentsAsHtml */
-  deprecated predicate interpretsArgumentsAsHTML(Expr arg) { this.interpretsArgumentsAsHtml(arg) }
+  deprecated predicate interpretsArgumentsAsHTML(DataFlow::Node arg) { this.interpretsArgumentsAsHtml(arg) }
 }
 
 /**
+ * DEPRECATED: Use `DomPropertyWrite` instead.
  * An assignment to a property of a DOM object.
  */
-class DomPropWriteNode extends Assignment {
-  PropAccess lhs;
+deprecated class DomPropWriteNode extends Assignment {
+  DomPropertyWrite node;
 
-  DomPropWriteNode() {
-    lhs = this.getLhs() and
-    isDomValue(lhs.getBase())
-  }
+  DomPropWriteNode() { this.flow() = node }
 
   /**
    * Holds if the assigned value is interpreted as HTML.
    */
-  predicate interpretsValueAsHtml() {
-    lhs.getPropertyName() = "innerHTML" or
-    lhs.getPropertyName() = "outerHTML"
-  }
+  predicate interpretsValueAsHtml() { node.interpretsValueAsHtml() }
 
   /** DEPRECATED: Alias for interpretsValueAsHtml */
   deprecated predicate interpretsValueAsHTML() { this.interpretsValueAsHtml() }
 
+  /**
+   * Holds if the assigned value is interpreted as JavaScript via javascript: protocol.
+   */
+  predicate interpretsValueAsJavaScriptUrl() { node.interpretsValueAsJavaScriptUrl() }
+}
+
+/**
+ * An assignment to a property of a DOM object.
+ */
+class DomPropertyWrite extends DataFlow::Node instanceof DataFlow::PropWrite {
+  DomPropertyWrite() { isDomNode(super.getBase()) }
+
+  /**
+   * Holds if the assigned value is interpreted as HTML.
+   */
+  predicate interpretsValueAsHtml() {
+    super.getPropertyName() = "innerHTML" or
+    super.getPropertyName() = "outerHTML"
+  }
+
   /**
    * Holds if the assigned value is interpreted as JavaScript via javascript: protocol.
    */
   predicate interpretsValueAsJavaScriptUrl() {
-    lhs.getPropertyName() = DOM::getAPropertyNameInterpretedAsJavaScriptUrl()
+    super.getPropertyName() = DOM::getAPropertyNameInterpretedAsJavaScriptUrl()
   }
+
+  /**
+   * Gets the data flow node corresponding to the value being written,
+   */
+  DataFlow::Node getRhs() { result = super.getRhs() }
 }
 
 /**

javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll

@@ -130,12 +130,12 @@ module DomBasedXss {
   class DomSink extends Sink {
     DomSink() {
       // Call to a DOM function that inserts its argument into the DOM
-      any(DomMethodCallExpr call).interpretsArgumentsAsHtml(this.asExpr())
+      any(DomMethodCallNode call).interpretsArgumentsAsHtml(this)
       or
       // Assignment to a dangerous DOM property
-      exists(DomPropWriteNode pw |
+      exists(DomPropertyWrite pw |
         pw.interpretsValueAsHtml() and
-        this = DataFlow::valueNode(pw.getRhs())
+        this = pw.getRhs()
       )
       or
       // `html` or `source.html` properties of React Native `WebView`
@@ -159,7 +159,7 @@ module DomBasedXss {
       )
       or
       exists(DataFlow::MethodCallNode ccf |
-        isDomValue(ccf.getReceiver().asExpr()) and
+        isDomNode(ccf.getReceiver()) and
         ccf.getMethodName() = "createContextualFragment" and
         this = ccf.getArgument(0)
       )