From 35d81513741282f7d5df78cf05c1e0715c086ccc Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Tue, 11 Feb 2020 11:19:57 +0100
Subject: [PATCH] add a few arrary methods to TaintedPath.qll

---
 .../security/dataflow/TaintedPath.qll         | 35 ++++++++++++++++---
 .../dataflow/TaintedPathCustomizations.qll    |  6 ++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
index b46f7d508f7..70662bcf60d 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPath.qll
@@ -93,13 +93,38 @@ module TaintedPath {
         |
           name = argumentlessMethodName
         )
-        or
+      )
+      or
+      // array method calls of interest
+      exists(DataFlow::MethodCallNode mcn, string name | dst = mcn and mcn.calls(src, name) |
+        // A `str.split()` call can either split into path elements (`str.split("/")`) or split by some other string.
         name = "split" and
-        not exists(DataFlow::Node splitBy | splitBy = mcn.getArgument(0) |
-          splitBy.mayHaveStringValue("/") or
-          any(DataFlow::RegExpLiteralNode reg | reg.getRoot().getAMatchedString() = "/")
-              .flowsTo(splitBy)
+        (
+          if
+            exists(DataFlow::Node splitBy | splitBy = mcn.getArgument(0) |
+              splitBy.mayHaveStringValue("/") or
+              any(DataFlow::RegExpLiteralNode reg | reg.getRoot().getAMatchedString() = "/")
+                  .flowsTo(splitBy)
+            )
+          then
+            srclabel.(Label::PosixPath).canContainDotDotSlash() and
+            dstlabel instanceof Label::SplitPath
+          else srclabel = dstlabel
         )
+        or
+        (
+          name = "pop" or
+          name = "shift" or
+          name = "slice" or
+          name = "splice"
+        ) and
+        dstlabel instanceof Label::SplitPath and
+        srclabel instanceof Label::SplitPath
+        or
+        name = "join" and
+        mcn.getArgument(0).mayHaveStringValue("/") and 
+        srclabel instanceof Label::SplitPath and
+        dstlabel.(Label::PosixPath).canContainDotDotSlash()
       )
     }
 
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
index 25bb232f8fe..cbb62d7b95d 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
@@ -108,6 +108,12 @@ module TaintedPath {
         not (isNormalized() and isAbsolute())
       }
     }
+
+    class SplitPath extends DataFlow::FlowLabel {
+      SplitPath() {
+        this = "splitPath"
+      }
+    }
   }
 
   /**