Squished changes for HttpToFileAccess commint

javascript/ql/src/Security/CWE-200/FileAccessToHttp.ql

@@ -0,0 +1,16 @@
+/**
+ * @name File Access data flows to Http POST/PUT 
+ * @description Writing data from file directly to http body or request header can be an indication to data exfiltration or unauthorized information disclosure.
+ * @kind problem
+ * @problem.severity warning
+ * @id js/file-access-to-http
+ * @tags security
+ *       external/cwe/cwe-200
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.FileAccessToHttp
+
+from FileAccessToHttpDataFlow::Configuration config, DataFlow::Node src, DataFlow::Node sink
+where config.hasFlow (src, sink)
+select src, "$@ flows directly to Http request body", sink, "File access"

javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql

@@ -0,0 +1,16 @@
+/**
+ * @name Http response data flows to File Access 
+ * @description Writing data from an HTTP request directly to the file system allows arbitrary file upload and might indicate a backdoor.
+ * @kind problem
+ * @problem.severity warning
+ * @id js/http-to-file-access
+ * @tags security
+ *       external/cwe/cwe-912
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.HttpToFileAccess
+
+from HttpToFileAccessFlow::Configuration configuration, DataFlow::Node src, DataFlow::Node sink
+where configuration.hasFlow(src, sink)
+select sink, "$@ flows to file system", src, "Untrusted data received from Http response"

javascript/ql/src/semmle/javascript/Concepts.qll

@@ -22,13 +22,27 @@ abstract class SystemCommandExecution extends DataFlow::Node {
 }
 
 /**
- * A data flow node that performs a file system access.
+ * A data flow node that performs a file system access (read, write, copy, permissions, stats, etc).
  */
 abstract class FileSystemAccess extends DataFlow::Node {
+
   /** Gets an argument to this file system access that is interpreted as a path. */
   abstract DataFlow::Node getAPathArgument();
+  
+  /** Gets a node that represents file system access data, such as buffer the data is copied to. */
+  abstract DataFlow::Node getDataNode();
 }
 
+/**
+ * A data flow node that performs read file system access.
+ */
+abstract class FileSystemReadAccess extends FileSystemAccess { }
+
+/**
+ * A data flow node that performs write file system access.
+ */
+abstract class FileSystemWriteAccess extends FileSystemAccess { }
+
 /**
  * A data flow node that contains a file name or an array of file names from the local file system.
  */

javascript/ql/src/semmle/javascript/frameworks/Express.qll

@@ -788,6 +788,10 @@ module Express {
       asExpr().(MethodCallExpr).calls(any(ResponseExpr res), "sendFile")
     }
 
+    override DataFlow::Node getDataNode() {
+      result = DataFlow::valueNode(astNode)
+    }
+
     override DataFlow::Node getAPathArgument() {
       result = DataFlow::valueNode(astNode.getArgument(0))
     }

javascript/ql/src/semmle/javascript/frameworks/HTTP.qll

@@ -132,6 +132,11 @@ module HTTP {
     result = "http" or result = "https"
   }
 
+  /**
+   * An expression whose value is sent as (part of) the body of an HTTP request (POST, PUT).
+   */
+  abstract class RequestBody extends DataFlow::Node {}
+  
   /**
    * An expression whose value is sent as (part of) the body of an HTTP response.
    */

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

@@ -336,6 +336,23 @@ module NodeJSLib {
     )
   }
 
+  /**
+   * Holds if the `i`th parameter of method `methodName` of the Node.js
+   * `fs` module might represent a data parameter or buffer or a callback 
+   * that receives the data.
+   *
+   * We determine this by looking for an externs declaration for
+   * `fs.methodName` where the `i`th parameter's name is `data` or
+   * `buffer` or a 'callback'.
+   */
+  private predicate fsDataParam(string methodName, int i, string n) {
+    exists (ExternalMemberDecl decl, Function f, JSDocParamTag p |
+      decl.hasQualifiedName("fs", methodName) and f = decl.getInit() and
+      p.getDocumentedParameter() = f.getParameter(i).getAVariable() and
+      n = p.getName().toLowerCase() |
+      n = "data" or n = "buffer" or n = "callback"
+    )
+  }
   /**
    * A member `member` from module `fs` or its drop-in replacements `graceful-fs` or `fs-extra`.
    */
@@ -348,21 +365,161 @@ module NodeJSLib {
     )
   }
 
+  
   /**
    * A call to a method from module `fs`, `graceful-fs` or `fs-extra`.
    */
-  private class NodeJSFileSystemAccess extends FileSystemAccess, DataFlow::CallNode {
+  private class NodeJSFileSystemAccessCall extends FileSystemAccess, DataFlow::CallNode {
     string methodName;
 
-    NodeJSFileSystemAccess() {
+    NodeJSFileSystemAccessCall() {
       this = fsModuleMember(methodName).getACall()
     }
 
+    string getMethodName() {
+        result = methodName
+    }
+
+    override DataFlow::Node getDataNode() {
+      (
+        methodName = "readFileSync" and 
+        result = this
+      ) 
+      or
+      exists (int i, string paramName | fsDataParam(methodName, i, paramName) |
+      (
+        paramName = "callback" and
+        exists (DataFlow::ParameterNode p, string n | 
+          p = getCallback(i).getAParameter() and
+          n = p.getName().toLowerCase() and
+          result = p |
+          n = "data" or n = "buffer" or n = "string"
+        )
+      ) 
+      or
+        result = getArgument(i))
+    }
+
     override DataFlow::Node getAPathArgument() {
-      exists (int i | fsFileParam(methodName, i) |
-        result = getArgument(i)
+      exists (int i | fsFileParam(methodName, i) | 
+        result = getArgument(i))
+    }
+  }
+
+  /** Only NodeJSSystemFileAccessCalls that write data to 'fs' */
+  private class NodeJSFileSystemAccessWriteCall extends FileSystemWriteAccess, NodeJSFileSystemAccessCall {
+      NodeJSFileSystemAccessWriteCall () {
+          this.getMethodName() = "appendFile" or
+          this.getMethodName() = "appendFileSync" or
+          this.getMethodName() = "write" or
+          this.getMethodName() = "writeFile" or
+          this.getMethodName() = "writeFileSync" or
+          this.getMethodName() = "writeSync"
+      }
+  }
+
+  /** Only NodeJSSystemFileAccessCalls that read data from 'fs' */
+  private class NodeJSFileSystemAccessReadCall extends FileSystemReadAccess, NodeJSFileSystemAccessCall {
+      NodeJSFileSystemAccessReadCall () {
+          this.getMethodName() = "read" or
+          this.getMethodName() = "readSync" or
+          this.getMethodName() = "readFile" or
+          this.getMethodName() = "readFileSync" 
+      }
+  }
+  
+  /**
+   * A call to write corresponds to a pattern where file stream is open first with 'createWriteStream', followed by 'write' or 'end' call
+   */
+   private class NodeJSFileSystemWrite extends FileSystemWriteAccess, DataFlow::CallNode {
+
+      NodeJSFileSystemAccessCall init;
+
+      NodeJSFileSystemWrite() {
+        exists (NodeJSFileSystemAccessCall n |
+            n.getCalleeName() = "createWriteStream" and init = n |
+            this = n.getAMemberCall("write") or 
+            this = n.getAMemberCall("end")
+        )
+      }
+      
+      override DataFlow::Node getDataNode() {
+          result = this.getArgument(0)
+      } 
+
+      override DataFlow::Node getAPathArgument() {
+          result = init.getAPathArgument()
+      }
+   }
+  
+  /**
+   * A call to read corresponds to a pattern where file stream is open first with createReadStream, followed by 'read' call
+   */
+   private class NodeJSFileSystemRead extends FileSystemReadAccess, DataFlow::CallNode {
+
+      NodeJSFileSystemAccessCall init;
+
+      NodeJSFileSystemRead() {
+        exists (NodeJSFileSystemAccessCall n | 
+            n.getCalleeName() = "createReadStream" and init = n |
+            this = n.getAMemberCall("read")
+        )
+      }
+
+      override DataFlow::Node getDataNode() {
+        result = this
+      }
+
+      override DataFlow::Node getAPathArgument() {
+          result = init.getAPathArgument()
+      }
+   }
+
+   /**
+   * A call to read corresponds to a pattern where file stream is open first with createReadStream, followed by 'pipe' call
+   */
+   private class NodeJSFileSystemPipe extends FileSystemReadAccess, DataFlow::CallNode {
+
+      NodeJSFileSystemAccessCall init;
+
+      NodeJSFileSystemPipe() {
+        exists (NodeJSFileSystemAccessCall n | 
+            n.getCalleeName() = "createReadStream" and init = n |
+            this = n.getAMemberCall("pipe")
+        )
+      }
+
+      override DataFlow::Node getDataNode() {
+        result = this.getArgument(0)
+      }
+
+      override DataFlow::Node getAPathArgument() {
+          result = init.getAPathArgument()
+      }
+   }
+   
+  /** 
+   * An 'on' event where data comes in as a parameter (usage: readstream.on('data', chunk)) 
+   */
+  private class NodeJSFileSystemReadDataEvent extends FileSystemReadAccess, DataFlow::CallNode {
+
+    NodeJSFileSystemAccessCall init;
+
+    NodeJSFileSystemReadDataEvent() {
+      exists(NodeJSFileSystemAccessCall n |
+        n.getCalleeName() = "createReadStream" and init = n |
+        this = n.getAMethodCall("on") and
+        this.getArgument(0).mayHaveStringValue("data")
       )
     }
+
+    override DataFlow::Node getDataNode() {
+        result = this.getCallback(1).getParameter(0)
+    }
+    
+    override DataFlow::Node getAPathArgument() {
+      result = init.getAPathArgument()
+    }
   }
 
   /**
@@ -602,7 +759,18 @@ module NodeJSLib {
     }
   }
   
-  
+  /**
+   * An argument to client request.write () method, can be used to write body to a HTTP or HTTPS POST/PUT request, 
+   * or request option (like headers, cookies, even url) 
+   */
+  class HttpRequestWriteArgument extends HTTP::RequestBody, DataFlow::Node {
+    HttpRequestWriteArgument () {
+      exists(CustomClientRequest req |
+        this = req.getAMethodCall("write").getArgument(0) or
+        this = req.getArgument(0))
+    }
+  }
+
   /**
    * A data flow node that is registered as a callback for an HTTP or HTTPS request made by a Node.js process, for example the function `handler` in `http.request(url).on(message, handler)`.
    */

javascript/ql/src/semmle/javascript/frameworks/Request.qll

@@ -44,5 +44,13 @@ module Request {
     }
 
   }
+  
+  // using 'request' library to make http 'POST' and 'PUT' requests with message body. 
+  private class RequestPostBody extends HTTP::RequestBody {
+      RequestPostBody () {
+          this = DataFlow::moduleMember("request", "post").getACall().getArgument(1) or
+          this = DataFlow::moduleImport("request").getAnInvocation().getArgument(0)
+      }
+  }
 
 }

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

@@ -0,0 +1,71 @@
+/** 
+ * Provides Taint tracking configuration for reasoning about file access taint flow to http post body 
+ */
+import javascript
+import semmle.javascript.frameworks.HTTP
+
+module FileAccessToHttpDataFlow {
+  /**
+   * A data flow source for reasoning about file access to http post body flow vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for reasoning about file access to http post body flow vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for reasoning about file access to http post body flow vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A taint-tracking configuration for reasoning about file access to http post body flow vulnerabilities.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "FileAccessToHttpDataFlow" }
+
+    override predicate isSource(DataFlow::Node source) {
+      source instanceof Source
+    }
+
+    override predicate isSink(DataFlow::Node sink) {
+      sink instanceof Sink
+    }
+
+    override predicate isSanitizer(DataFlow::Node node) {
+      super.isSanitizer(node) or
+      node instanceof Sanitizer
+    }
+    
+    /** additional taint step that taints an object wrapping a source */
+    override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+      (
+          pred = DataFlow::valueNode(_) or
+          pred = DataFlow::parameterNode(_) or
+          pred instanceof DataFlow::PropRead
+      ) and
+      exists (DataFlow::PropWrite pwr |
+        succ = pwr.getBase() and
+        pred = pwr.getRhs()
+      )
+    }
+  }
+
+  /** A source is a file access parameter, as in readFromFile(buffer). */
+  private class FileAccessArgumentAsSource extends Source {
+    FileAccessArgumentAsSource() {  
+      exists(FileSystemReadAccess src |
+        this = src.getDataNode().getALocalSource()
+      )
+    }
+  }
+
+  /** Sink is any parameter or argument that evaluates to a parameter ot a function or call that sets Http Body on a request */ 
+  private class HttpRequestBodyAsSink extends Sink {
+    HttpRequestBodyAsSink () {
+      this instanceof HTTP::RequestBody
+    }
+  }
+}

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

@@ -0,0 +1,56 @@
+/** 
+ * Provides taint tracking configuration for reasoning about files created from untrusted http downloads.  
+ */
+import javascript
+import semmle.javascript.security.dataflow.RemoteFlowSources
+
+module HttpToFileAccessFlow {
+  /**
+   * A data flow source from untrusted http request to file access taint tracking configuration.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for untrusted http request to file access taint tracking configuration.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for untrusted http request to file access taint tracking configuration.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A taint-tracking configuration for reasoning about file access from untrusted http response body.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "HttpToFileAccessFlow" }
+
+    override predicate isSource(DataFlow::Node source) {
+      source instanceof Source
+    }
+
+    override predicate isSink(DataFlow::Node sink) {
+      sink instanceof Sink
+    }
+
+    override predicate isSanitizer(DataFlow::Node node) {
+      super.isSanitizer(node) or
+      node instanceof Sanitizer
+    }
+  }
+  
+  /** A source of remote data, considered as a flow source for untrusted http data to file system access. */
+  class RemoteFlowSourceAsSource extends Source {
+    RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
+  }
+  
+  /** A sink that represents file access method (write, append) argument */
+  class FileAccessAsSink extends Sink {
+    FileAccessAsSink () {
+      exists(FileSystemWriteAccess src |
+        this = src.getDataNode()
+      )
+    }
+  }
+}