Merge pull request #1360 from asger-semmle/customize-window-document

JS: Make some DOM concepts customizable

Author: Max Schaefer

javascript/ql/src/semmle/javascript/DOM.qll

@@ -284,12 +284,25 @@ module DOM {
     )
   }
 
+  module DomValueSource {
+    /**
+     * A data flow node that should be considered a source of DOM values.
+     */
+    abstract class Range extends DataFlow::Node {}
+
+    private class DefaultRange extends Range {
+      DefaultRange() {
+        this.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
+        this = domValueRef().getAPropertyRead() or
+        this = domElementCreationOrQuery() or
+        this = domElementCollection()
+      }
+    }
+  }
+
   /** Gets a data flow node that refers directly to a value from the DOM. */
   DataFlow::SourceNode domValueSource() {
-    result.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
-    result = domValueRef().getAPropertyRead() or
-    result = domElementCreationOrQuery() or
-    result = domElementCollection()
+    result instanceof DomValueSource::Range
   }
 
   /** Gets a data flow node that may refer to a value from the DOM. */
@@ -303,11 +316,34 @@ module DOM {
   /** Gets a data flow node that may refer to a value from the DOM. */
   DataFlow::SourceNode domValueRef() { result = domValueRef(DataFlow::TypeTracker::end()) }
 
+  module LocationSource {
+    /**
+     * A data flow node that should be considered a source of the DOM `location` object.
+     *
+     * Can be subclassed to add additional such nodes.
+     */
+    abstract class Range extends DataFlow::Node {}
+
+    private class DefaultRange extends Range {
+      DefaultRange() {
+        exists(string propName | this = documentRef().getAPropertyRead(propName) |
+          propName = "documentURI" or
+          propName = "documentURIObject" or
+          propName = "location" or
+          propName = "referrer" or
+          propName = "URL"
+        )
+        or
+        this = DOM::domValueRef().getAPropertyRead("baseUri")
+        or
+        this = DataFlow::globalVarRef("location")
+      }
+    }
+  }
+
   /** Gets a data flow node that directly refers to a DOM `location` object. */
   DataFlow::SourceNode locationSource() {
-    result = domValueRef().getAPropertyRead("location")
-    or
-    result = DataFlow::globalVarRef("location")
+    result instanceof LocationSource::Range
   }
 
   /** Gets a reference to a DOM `location` object. */
@@ -321,12 +357,32 @@ module DOM {
   /** Gets a reference to a DOM `location` object. */
   DataFlow::SourceNode locationRef() { result = locationRef(DataFlow::TypeTracker::end()) }
 
+  module DocumentSource {
+    /**
+     * A data flow node that should be considered a source of the `document` object.
+     *
+     * Can be subclassed to add additional such nodes.
+     */
+    abstract class Range extends DataFlow::Node {}
+
+    private class DefaultRange extends Range {
+      DefaultRange() { this = DataFlow::globalVarRef("document") }
+    }
+  }
+
+  /**
+   * Gets a direct reference to the `document` object.
+   */
+  DataFlow::SourceNode documentSource() {
+    result instanceof DocumentSource::Range
+  }
+
   /**
    * Gets a reference to the `document` object.
    */
   private DataFlow::SourceNode documentRef(DataFlow::TypeTracker t) {
     t.start() and
-    result = DataFlow::globalVarRef("document")
+    result instanceof DocumentSource::Range
     or
     exists(DataFlow::TypeTracker t2 | result = documentRef(t2).track(t2, t))
   }

javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll

@@ -40,7 +40,7 @@ module ClientSideUrlRedirect {
     override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
     override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
-      isDocumentURL(source.asExpr()) and
+      source = DOM::locationSource() and
       lbl instanceof DocumentUrl
     }
 

javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll

@@ -51,8 +51,8 @@ module CodeInjection {
   /**
    * An access to a property that may hold (parts of) the document URL.
    */
-  class LocationSource extends Source, DataFlow::ValueNode {
-    LocationSource() { isDocumentURL(astNode) }
+  class LocationSource extends Source {
+    LocationSource() { this = DOM::locationSource() }
   }
 
   /**

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

@@ -38,17 +38,7 @@ predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
 
 /** Holds if `e` could refer to the document URL. */
 predicate isDocumentURL(Expr e) {
-  exists(string propName | e = DOM::documentRef().getAPropertyRead(propName).asExpr() |
-    propName = "documentURI" or
-    propName = "documentURIObject" or
-    propName = "location" or
-    propName = "referrer" or
-    propName = "URL"
-  )
-  or
-  e = DOM::domValueRef().getAPropertyRead("baseUri").asExpr()
-  or
-  e.accessesGlobal("location")
+  e.flow() = DOM::locationSource()
 }
 
 /**

javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll

@@ -42,7 +42,7 @@ module DomBasedXss {
   /**
    * An access of the URL of this page, or of the referrer to this page.
    */
-  class LocationSource extends Source, DataFlow::ValueNode {
-    LocationSource() { isDocumentURL(astNode) }
+  class LocationSource extends Source {
+    LocationSource() { this = DOM::locationSource() }
   }
 }

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccess.qll

@@ -110,7 +110,7 @@ module UnsafeDynamicMethodAccess {
    * The page URL considered as a flow source for unsafe dynamic method access.
    */
   class DocumentUrlAsSource extends Source {
-    DocumentUrlAsSource() { isDocumentURL(asExpr()) }
+    DocumentUrlAsSource() { this = DOM::locationSource() }
   }
 
   /**

javascript/ql/src/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCall.qll

@@ -102,7 +102,7 @@ module UnvalidatedDynamicMethodCall {
    * The page URL considered as a flow source for unvalidated dynamic method calls.
    */
   class DocumentUrlAsSource extends Source {
-    DocumentUrlAsSource() { isDocumentURL(asExpr()) }
+    DocumentUrlAsSource() { this = DOM::locationSource() }
   }
 
   /**

javascript/ql/src/semmle/javascript/security/dataflow/XpathInjection.qll

@@ -64,8 +64,8 @@ module XpathInjection {
   }
 
   /** A part of the document URL, considered as a flow source for XPath injection. */
-  class DocumentUrlSource extends Source, DataFlow::ValueNode {
-    DocumentUrlSource() { isDocumentURL(astNode) }
+  class DocumentUrlSource extends Source {
+    DocumentUrlSource() { this = DOM::locationSource() }
   }
 
   /**

javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll

@@ -70,7 +70,7 @@ module DomBasedXss {
           strval = prefix.getStringValue() and
           not strval.regexpMatch("\\s*<.*")
         ) and
-        not isDocumentURL(astNode)
+        not DOM::locationRef().flowsTo(this)
       )
       or
       // call to an Angular method that interprets its argument as HTML

javascript/ql/test/library-tests/DOM/Customizations.expected

@@ -0,0 +1,6 @@
+test_documentRef
+| customization.js:2:13:2:31 | customGetDocument() |
+test_locationRef
+| customization.js:3:3:3:14 | doc.location |
+test_domValueRef
+| customization.js:4:3:4:28 | doc.get ... 'test') |