add Dice package, add global taint steps by SharedTaintStep, use getASuccessor

Author: am0o0

javascript/ql/lib/semmle/javascript/frameworks/FormParsers.qll

@@ -38,17 +38,18 @@ module BusBoy {
   }
 
   /**
-   * Holds if busboy file data as additional taint steps according to a Readable Stream type
-   *
-   * TODO: I don't know how it can be a global taint step!
+   * A busboy file data step according to a Readable Stream type
    */
-  predicate busBoyReadableAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(API::Node busboyOnEvent |
-      busboyOnEvent = API::moduleImport("busboy").getReturn().getMember("on")
-    |
-      busboyOnEvent.getParameter(0).asSink().mayHaveStringValue("file") and
-      customStreamPipeAdditionalTaintStep(busboyOnEvent.getParameter(1).getParameter(1), pred, succ)
-    )
+  private class AdditionalTaintStep extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(API::Node busboyOnEvent |
+        busboyOnEvent = API::moduleImport("busboy").getReturn().getMember("on")
+      |
+        busboyOnEvent.getParameter(0).asSink().mayHaveStringValue("file") and
+        customStreamPipeAdditionalTaintStep(busboyOnEvent.getParameter(1).getParameter(1), pred,
+          succ)
+      )
+    }
   }
 }
 
@@ -89,11 +90,6 @@ module Formidable {
   }
 }
 
-API::Node test() {
-  result =
-    API::moduleImport("multiparty").getMember("Form").getInstance().getMember("on").getASuccessor*()
-}
-
 /**
  * A module for modeling [multiparty](https://www.npmjs.com/package/multiparty) package
  */
@@ -116,7 +112,7 @@ module Multiparty {
             this = on.getParameter(1).getParameter([0, 1]).asSource()
             or
             on.getParameter(0).asSink().mayHaveStringValue("part") and
-            this = readableStreamDataNode(on.getParameter(1).getParameter(0)).asSink()
+            this = readableStreamDataNode(on.getParameter(1).getParameter(0)).asSource()
           )
         )
       )
@@ -126,18 +122,58 @@ module Multiparty {
   }
 
   /**
-   * Holds if multiparty part data as additional taint steps according to a Readable Stream type
-   *
-   * TODO: I don't know how it can be a global taint step!
+   * A multiparty part data step according to a Readable Stream type
+   */
+  private class AdditionalTaintStep extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(API::Node multipartyOnEvent |
+        multipartyOnEvent =
+          API::moduleImport("multiparty").getMember("Form").getInstance().getMember("on")
+      |
+        multipartyOnEvent.getParameter(0).asSink().mayHaveStringValue("part") and
+        customStreamPipeAdditionalTaintStep(multipartyOnEvent.getParameter(1).getParameter(0), pred,
+          succ)
+      )
+    }
+  }
+}
+
+/**
+ * A module for modeling [dicer](https://www.npmjs.com/package/dicer) package
+ */
+module Dicer {
+  /**
+   * A source of remote flow from the `dicer` library.
    */
-  predicate multipartyReadableAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(API::Node multipartyOnEvent |
-      multipartyOnEvent =
-        API::moduleImport("multiparty").getMember("Form").getInstance().getMember("on")
-    |
-      multipartyOnEvent.getParameter(0).asSink().mayHaveStringValue("part") and
-      customStreamPipeAdditionalTaintStep(multipartyOnEvent.getParameter(1).getParameter(0), pred,
-        succ)
-    )
+  private class DicerRemoteFlow extends RemoteFlowSource {
+    DicerRemoteFlow() {
+      exists(API::Node dicer | dicer = API::moduleImport("dicer").getInstance() |
+        exists(API::Node on | on = dicer.getMember("on") |
+          on.getParameter(0).asSink().mayHaveStringValue("part") and
+          this = readableStreamDataNode(on.getParameter(1).getParameter(0)).asSource()
+          or
+          exists(API::Node onPart | onPart = on.getParameter(1).getParameter(0).getMember("on") |
+            onPart.getParameter(0).asSink().mayHaveStringValue("header") and
+            this = onPart.getParameter(1).getParameter(0).asSource()
+          )
+        )
+      )
+    }
+
+    override string getSourceType() { result = "parsed user value from Dicer" }
+  }
+
+  /**
+   * A dicer part data step according to a Readable Stream type
+   */
+  private class AdditionalTaintStep extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(API::Node onEvent |
+        onEvent = API::moduleImport("dicer").getInstance().getMember("on")
+      |
+        onEvent.getParameter(0).asSink().mayHaveStringValue("part") and
+        customStreamPipeAdditionalTaintStep(onEvent.getParameter(1).getParameter(0), pred, succ)
+      )
+    }
   }
 }

javascript/ql/lib/semmle/javascript/frameworks/ReadableStream.qll

@@ -118,8 +118,6 @@ API::Node nodeJsStream() {
  * and returns all nodes responsible for a data read access
  */
 API::Node readableStreamDataNode(API::Node stream) {
-  result = stream.getMember("read").getReturn()
-  or
   // 'data' event
   exists(API::CallNode onEvent | onEvent = stream.getMember("on").getACall() |
     result = onEvent.getParameter(1).getParameter(0) and
@@ -128,7 +126,10 @@ API::Node readableStreamDataNode(API::Node stream) {
   or
   // 'Readable' event
   exists(API::CallNode onEvent | onEvent = stream.getMember("on").getACall() |
-    result = onEvent.getParameter(1).getReceiver().getMember("read").getReturn() and
+    (
+      result = onEvent.getParameter(1).getReceiver().getMember("read").getReturn() or
+      result = stream.getMember("read").getReturn()
+    ) and
     onEvent.getParameter(0).asSink().mayHaveStringValue("readable")
   )
 }

javascript/ql/src/experimental/Security/CWE-522-DecompressionBombs/DecompressionBombs.ql

@@ -215,19 +215,7 @@ module DecompressionBomb {
         result =
           this.getMember(["extractAllTo", "extractEntryTo", "readAsText"]).getReturn().asSource()
         or
-        result = this.getAAdmZipSuccessor().getMember("getData").getReturn().asSource()
-      }
-
-      API::Node getAAdmZipSuccessor() {
-        result = this
-        or
-        result = this.getAAdmZipSuccessor().getAMember()
-        or
-        result = this.getAAdmZipSuccessor().getAParameter()
-        or
-        result = this.getAAdmZipSuccessor().getReturn()
-        or
-        result = this.getAAdmZipSuccessor().getPromised()
+        result = this.getASuccessor*().getMember("getData").getReturn().asSource()
       }
     }
 
@@ -244,7 +232,6 @@ module DecompressionBomb {
                   .getReturn()
                   .asSource()
             or
-            // I can't find an alternative for getASuccessor*() for here
             succ =
               n.getInstance()
                   .getMember("getEntries")
@@ -340,21 +327,7 @@ module DecompressionBomb {
        * so i'm going to check if there is a member like `vars.uncompressedSize` in whole DB or not!
        */
       predicate sanitizer() {
-        exists(this.getAGzipperSuccessor().getMember("vars").getMember("uncompressedSize"))
-      }
-
-      API::Node getAGzipperSuccessor() {
-        (
-          result = API::moduleImport("stream")
-          or
-          result = this.getAGzipperSuccessor().getAMember()
-          or
-          result = this.getAGzipperSuccessor().getAParameter()
-          or
-          result = this.getAGzipperSuccessor().getReturn()
-          or
-          result = this.getAGzipperSuccessor().getPromised()
-        ) and
+        exists(this.getASuccessor*().getMember("vars").getMember("uncompressedSize")) and
         funcName = ["Extract", "Parse", "ParseOne"]
       }
     }
@@ -381,35 +354,20 @@ module DecompressionBomb {
       }
 
       override DataFlow::Node sink() {
-        result = this.getAYauzlSuccessor().getMember("readEntry").getACall() and
+        result = this.getASuccessor*().getMember("readEntry").getACall() and
         not this.sanitizer() and
         isOpenFunc = true
         or
         result = this.getACall().getArgument(0) and
         isOpenFunc = false
       }
 
-      API::Node getAYauzlSuccessor() {
-        (
-          result = this
-          or
-          result = this.getAYauzlSuccessor().getAMember()
-          or
-          result = this.getAYauzlSuccessor().getAParameter()
-          or
-          result = this.getAYauzlSuccessor().getReturn()
-          or
-          result = this.getAYauzlSuccessor().getPromised()
-        ) and
-        isOpenFunc = true
-      }
-
       /**
        * Gets a
        * and Holds if yauzl `open` instance has a member `uncompressedSize`
        */
       predicate sanitizer() {
-        exists(this.getAYauzlSuccessor().getMember("uncompressedSize")) and
+        exists(this.getASuccessor*().getMember("uncompressedSize")) and
         isOpenFunc = true
       }
     }