Merge pull request #17951 from Napalys/napalys/reverse-support

JS: Added support for reverse function
github · Nov 12, 2024 · 6266dab · 6266dab
2 parents ba26281 + 42f7f73
commit 6266dab
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 13 deletions.
diff --git a/javascript/ql/lib/change-notes/2024-11-11-reserve-support.md b/javascript/ql/lib/change-notes/2024-11-11-reserve-support.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added taint-steps for `Array.prototype.reverse`
diff --git a/javascript/ql/lib/semmle/javascript/Arrays.qll b/javascript/ql/lib/semmle/javascript/Arrays.qll
@@ -444,4 +444,18 @@ private module ArrayLibraries {
       )
     }
   }
+
+  /**
+   * A taint propagating data flow edge arising from in-place array manipulation operations.
+   * The methods return the pointer to `this` array as well.
+   */
+  private class ArrayInPlaceManipulationTaintStep extends TaintTracking::SharedTaintStep {
+    override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::MethodCallNode call |
+        call.getMethodName() in ["sort", "reverse"] and
+        pred = call.getReceiver() and
+        succ = call
+      )
+    }
+  }
 }
diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
@@ -869,19 +869,6 @@ module TaintTracking {
     }
   }
 
-  /**
-   * A taint propagating data flow edge arising from sorting.
-   */
-  private class SortTaintStep extends SharedTaintStep {
-    override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(DataFlow::MethodCallNode call |
-        call.getMethodName() = "sort" and
-        pred = call.getReceiver() and
-        succ = call
-      )
-    }
-  }
-
   /**
    * A taint step through an exception constructor, such as `x` to `new Error(x)`.
    */

diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
@@ -233,6 +233,7 @@ typeInferenceMismatch
 | tst.js:2:13:2:20 | source() | tst.js:48:10:48:22 | new Buffer(x) |
 | tst.js:2:13:2:20 | source() | tst.js:51:10:51:31 | seriali ... ript(x) |
 | tst.js:2:13:2:20 | source() | tst.js:54:14:54:19 | unsafe |
+| tst.js:2:13:2:20 | source() | tst.js:61:10:61:20 | x.reverse() |
 | xml.js:5:18:5:25 | source() | xml.js:8:14:8:17 | text |
 | xml.js:12:17:12:24 | source() | xml.js:13:14:13:19 | result |
 | xml.js:23:18:23:25 | source() | xml.js:20:14:20:17 | attr |

diff --git a/javascript/ql/test/library-tests/TaintTracking/tst.js b/javascript/ql/test/library-tests/TaintTracking/tst.js
@@ -57,4 +57,6 @@ function test() {
     }
 
     tagged`foo ${"safe"} bar ${x} baz`;
+
+    sink(x.reverse()); // NOT OK
 }