change the pseudo-property on URL to a two-stage process

Author: erik-krogh

javascript/ql/src/Security/CWE-079/Xss.actual

@@ -0,0 +1,3 @@
+nodes
+edges
+#select

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -615,13 +615,21 @@ module TaintTracking {
   }
 
   /**
-   * A pseudo-property used to store a value on a `URLSearchParams` that 
-   * can be obtained with a `get` or `getAll` call. 
+   * A pseudo-property a `URL` that stores a value that can be obtained
+   * with a `get` or `getAll` call to the `searchParams` property. 
    */
   private string hiddenUrlPseudoProperty() {
     result = "$hiddenSearchPararms"
   }
 
+  /**
+   * A pseudo-property on a `URLSearchParams` that can be obtained
+   * with a `get` or `getAll` call. 
+   */ 
+  private string getableUrlPseudoProperty() {
+    result = "$gettableSearchPararms"
+  }
+
   /**
    * A taint propagating data flow edge arising from URL parameter parsing.
    */
@@ -654,17 +662,18 @@ module TaintTracking {
         pred = newUrl.getArgument(0)
       )
       or
-      prop = hiddenUrlPseudoProperty() and
+      prop = getableUrlPseudoProperty() and
       isUrlSearchParams(succ, pred)
     }
 
     /**
-     * Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
+     * Holds if the property `loadStep` should be copied from the object `pred` to the property `storeStep` of object `succ`.
      * 
      * This step is used to copy a value the value of our pseudo-property that can later be accessed using a `get` or `getAll` call. 
      */
-    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      prop = hiddenUrlPseudoProperty() and
+    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string loadProp, string storeProp) {
+      loadProp = hiddenUrlPseudoProperty() and
+      storeProp = getableUrlPseudoProperty() and
       exists(DataFlow::PropRead write | write = succ | 
         write.getPropertyName() = "searchParams" and
         write.getBase() = pred
@@ -677,7 +686,7 @@ module TaintTracking {
     * This step is used to load the value stored in the hidden pseudo-property. 
     */
     override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) { 
-      prop = hiddenUrlPseudoProperty() and
+      prop = getableUrlPseudoProperty() and
       // this is a call to `get` or `getAll` on a `URLSearchParams` object
       exists(string m, DataFlow::MethodCallNode call | call = succ |
         call.getMethodName() = m and

javascript/ql/test/query-tests/Security/CWE-079/tst.js

@@ -335,4 +335,8 @@ function URLPseudoProperties() {
   let params = getTaintedUrl().searchParams;
   $('name').html(params.get('name'));
 
+  // OK (.get is not defined on a URL)
+  let myUrl = getTaintedUrl();
+  $('name').html(myUrl.get('name'));
+
 }