Merge pull request #310 from esben-semmle/js/additional-client-request-data-nodes

Approved by xiemaisi
2026-04-28 02:05:14 +02:00 · 2018-10-16 12:59:22 +01:00
parent e319159a59 870811a509
commit e55eaefded
14 changed files with 139 additions and 42 deletions
--- a/javascript/ql/src/semmle/javascript/dataflow/Sources.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/Sources.qll
@@ -131,6 +131,26 @@ abstract class SourceNode extends DataFlow::Node {
    )
  }

+  /**
+   * Gets a method call that invokes a method on this node.
+   *
+   * This includes only calls that have the syntactic shape of a method call,
+   * that is, `o.m(...)` or `o[p](...)`.
+   */
+  DataFlow::CallNode getAMethodCall() {
+    result = getAMethodCall(_)
+  }
+
+  /**
+   * Gets a chained method call that invokes `methodName` last.
+   *
+   * The chain steps include only calls that have the syntactic shape of a method call,
+   * that is, `o.m(...)` or `o[p](...)`.
+   */
+  DataFlow::CallNode getAChainedMethodCall(string methodName) {
+    result = getAMethodCall*().getAMethodCall(methodName)
+  }
+
  /**
   * Gets a `new` call that invokes constructor `constructorName` on this node.
   */
--- a/javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/ClientRequests.qll
@@ -116,31 +116,36 @@ private class RequestUrlRequest extends CustomClientRequest {
 */
 private class AxiosUrlRequest extends CustomClientRequest {

-  DataFlow::Node url;
+  string method;

  AxiosUrlRequest() {
    exists (string moduleName, DataFlow::SourceNode callee |
      this = callee.getACall() |
      moduleName = "axios" and
      (
-        callee = DataFlow::moduleImport(moduleName) or
-        callee = DataFlow::moduleMember(moduleName, httpMethodName()) or
-        callee = DataFlow::moduleMember(moduleName, "request")
-      ) and
-      (
-        url = getArgument(0) or
-        // depends on the method name and the call arity, over-approximating slightly in the name of simplicity
-        url = getOptionArgument([0..2], urlPropertyName())
+        callee = DataFlow::moduleImport(moduleName) and method = "request" or
+        callee = DataFlow::moduleMember(moduleName, method) and (method = httpMethodName() or method = "request")
      )
    )
  }

  override DataFlow::Node getUrl() {
-    result = url
+    result = getArgument(0) or
+    // depends on the method name and the call arity, over-approximating slightly in the name of simplicity
+    result = getOptionArgument([0..2], urlPropertyName())
  }

  override DataFlow::Node getADataNode() {
-    none()
+    method = "request" and
+    result = getOptionArgument(0, "data")
+    or
+    (method = "post" or method = "put" or method = "put") and
+    (result = getArgument(1) or result = getOptionArgument(2, "data"))
+    or
+    exists (string name |
+      name = "headers" or name = "params"|
+      result = getOptionArgument([0..2], name)
+    )
  }

 }
@@ -175,7 +180,10 @@ private class FetchUrlRequest extends CustomClientRequest {
  }

  override DataFlow::Node getADataNode() {
-    none()
+    exists (string name |
+      name = "headers" or name = "body" |
+      result = getOptionArgument(1, name)
+    )
  }

 }
@@ -185,8 +193,6 @@ private class FetchUrlRequest extends CustomClientRequest {
 */
 private class GotUrlRequest extends CustomClientRequest {

-  DataFlow::Node url;
-
  GotUrlRequest() {
    exists (string moduleName, DataFlow::SourceNode callee |
      this = callee.getACall() |
@@ -194,17 +200,20 @@ private class GotUrlRequest extends CustomClientRequest {
      (
        callee = DataFlow::moduleImport(moduleName) or
        callee = DataFlow::moduleMember(moduleName, "stream")
-      ) and
-      url = getArgument(0) and not exists (getOptionArgument(1, "baseUrl"))
+      )
    )
  }

  override DataFlow::Node getUrl() {
-    result = url
+    result = getArgument(0) and
+    not exists (getOptionArgument(1, "baseUrl"))
  }

  override DataFlow::Node getADataNode() {
-    none()
+    exists (string name |
+      name = "headers" or name = "body" or name = "query" |
+      result = getOptionArgument(1, name)
+    )
  }

 }
@@ -230,7 +239,10 @@ private class SuperAgentUrlRequest extends CustomClientRequest {
  }

  override DataFlow::Node getADataNode() {
-    none()
+    exists (string name |
+      name = "set" or name = "send" or name = "query" |
+      result = this.getAChainedMethodCall(name).getAnArgument()
+    )
  }

 }
--- a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll
@@ -49,32 +49,14 @@ module Electron {
    }

  }
-  
-  /**
-   * A Node.js-style HTTP or HTTPS request made using `electron.net`, for example `net.request(url)`.
-   */
-  private class NetRequest extends CustomElectronClientRequest {
-    NetRequest() {
-      this = DataFlow::moduleMember("electron", "net").getAMemberCall("request")
-    }
-
-    override DataFlow::Node getUrl() {
-      result = getArgument(0) or
-      result = getOptionArgument(0, "url")
-    }
-
-    override DataFlow::Node getADataNode() {
-      none()
-    }
-
-  }

  /**
-   * A Node.js-style HTTP or HTTPS request made using `electron.client`, for example `new client(url)`.
+   * A Node.js-style HTTP or HTTPS request made using `electron.ClientRequest`.
   */
  private class NewClientRequest extends CustomElectronClientRequest {
    NewClientRequest() {
-      this = DataFlow::moduleMember("electron", "ClientRequest").getAnInstantiation()
+      this = DataFlow::moduleMember("electron", "ClientRequest").getAnInstantiation() or
+      this = DataFlow::moduleMember("electron", "net").getAMemberCall("request") // alias
    }

    override DataFlow::Node getUrl() {
@@ -83,7 +65,10 @@ module Electron {
    }

    override DataFlow::Node getADataNode() {
-      none()
+      exists (string name |
+        name = "write" or name = "end" |
+        result =this.(DataFlow::SourceNode).getAMethodCall(name).getArgument(0)
+      )
    }

  }
--- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll
@@ -740,7 +740,10 @@ module NodeJSLib {
    }

    override DataFlow::Node getADataNode() {
-      result = getAMethodCall("write").getArgument(0)
+      exists (string name |
+        name = "write" or name = "end" |
+        result =this.(DataFlow::SourceNode).getAMethodCall(name).getArgument(0)
+      )
    }

  }
--- a/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest.expected
+++ b/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest.expected
@@ -16,3 +16,14 @@
 | tst.js:41:5:41:29 | net.req ...  url }) |
 | tst.js:43:5:43:26 | new Cli ... st(url) |
 | tst.js:45:5:45:35 | new Cli ...  url }) |
+| tst.js:53:5:53:23 | axios({data: data}) |
+| tst.js:55:5:55:34 | axios.g ... _data}) |
+| tst.js:57:5:57:39 | axios.p ... data2}) |
+| tst.js:59:5:59:52 | axios({ ... sData}) |
+| tst.js:61:5:61:60 | window. ... yData}) |
+| tst.js:63:5:63:68 | got(url ... yData}) |
+| tst.js:65:5:65:23 | superagent.get(url) |
+| tst.js:66:5:66:23 | superagent.get(url) |
+| tst.js:67:5:67:24 | superagent.post(url) |
+| tst.js:68:5:68:23 | superagent.get(url) |
+| tst.js:69:5:69:23 | superagent.get(url) |
--- a/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getADataNode.expected
+++ b/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getADataNode.expected
@@ -0,0 +1,17 @@
+| tst.js:53:5:53:23 | axios({data: data}) | tst.js:53:18:53:21 | data |
+| tst.js:57:5:57:39 | axios.p ... data2}) | tst.js:57:19:57:23 | data1 |
+| tst.js:57:5:57:39 | axios.p ... data2}) | tst.js:57:33:57:37 | data2 |
+| tst.js:59:5:59:52 | axios({ ... sData}) | tst.js:59:21:59:30 | headerData |
+| tst.js:59:5:59:52 | axios({ ... sData}) | tst.js:59:41:59:50 | paramsData |
+| tst.js:61:5:61:60 | window. ... yData}) | tst.js:61:33:61:42 | headerData |
+| tst.js:61:5:61:60 | window. ... yData}) | tst.js:61:51:61:58 | bodyData |
+| tst.js:63:5:63:68 | got(url ... yData}) | tst.js:63:24:63:33 | headerData |
+| tst.js:63:5:63:68 | got(url ... yData}) | tst.js:63:42:63:49 | bodyData |
+| tst.js:65:5:65:23 | superagent.get(url) | tst.js:65:31:65:34 | data |
+| tst.js:66:5:66:23 | superagent.get(url) | tst.js:66:29:66:31 | 'x' |
+| tst.js:66:5:66:23 | superagent.get(url) | tst.js:66:34:66:43 | headerData |
+| tst.js:67:5:67:24 | superagent.post(url) | tst.js:67:31:67:38 | bodyData |
+| tst.js:68:5:68:23 | superagent.get(url) | tst.js:68:29:68:31 | 'x' |
+| tst.js:68:5:68:23 | superagent.get(url) | tst.js:68:34:68:43 | headerData |
+| tst.js:68:5:68:23 | superagent.get(url) | tst.js:68:52:68:60 | queryData |
+| tst.js:69:5:69:23 | superagent.get(url) | tst.js:69:48:69:56 | queryData |
--- a/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getADataNode.ql
+++ b/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getADataNode.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from ClientRequest r
+select r, r.getADataNode()
--- a/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getUrl.expected
+++ b/javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequest_getUrl.expected
@@ -20,3 +20,14 @@
 | tst.js:43:5:43:26 | new Cli ... st(url) | tst.js:43:23:43:25 | url |
 | tst.js:45:5:45:35 | new Cli ...  url }) | tst.js:45:23:45:34 | { url: url } |
 | tst.js:45:5:45:35 | new Cli ...  url }) | tst.js:45:30:45:32 | url |
+| tst.js:53:5:53:23 | axios({data: data}) | tst.js:53:11:53:22 | {data: data} |
+| tst.js:55:5:55:34 | axios.g ... _data}) | tst.js:55:15:55:15 | x |
+| tst.js:57:5:57:39 | axios.p ... data2}) | tst.js:57:16:57:16 | x |
+| tst.js:59:5:59:52 | axios({ ... sData}) | tst.js:59:11:59:51 | {header ... msData} |
+| tst.js:61:5:61:60 | window. ... yData}) | tst.js:61:18:61:20 | url |
+| tst.js:63:5:63:68 | got(url ... yData}) | tst.js:63:9:63:11 | url |
+| tst.js:65:5:65:23 | superagent.get(url) | tst.js:65:20:65:22 | url |
+| tst.js:66:5:66:23 | superagent.get(url) | tst.js:66:20:66:22 | url |
+| tst.js:67:5:67:24 | superagent.post(url) | tst.js:67:21:67:23 | url |
+| tst.js:68:5:68:23 | superagent.get(url) | tst.js:68:20:68:22 | url |
+| tst.js:69:5:69:23 | superagent.get(url) | tst.js:69:20:69:22 | url |
--- a/javascript/ql/test/library-tests/frameworks/ClientRequests/tst.js
+++ b/javascript/ql/test/library-tests/frameworks/ClientRequests/tst.js
@@ -48,3 +48,24 @@ import {ClientRequest, net} from 'electron';

    unknown({ url:url });
 });
+
+(function() {
+    axios({data: data});
+
+    axios.get(x, {data: not_data});
+
+    axios.post(x, data1, {data: data2});
+
+    axios({headers: headerData, params: paramsData});
+
+    window.fetch(url, {headers: headerData, body: bodyData});
+
+    got(url, {headers: headerData, body: bodyData, quer: queryData});
+
+    superagent.get(url).query(data);
+    superagent.get(url).set('x', headerData)
+    superagent.post(url).send(bodyData);
+    superagent.get(url).set('x', headerData).query(queryData);
+    superagent.get(url).unknown(nonData).query(queryData);
+
+});
--- a/javascript/ql/test/library-tests/frameworks/Electron/ClientRequest_getADataNode.expected
+++ b/javascript/ql/test/library-tests/frameworks/Electron/ClientRequest_getADataNode.expected
@@ -0,0 +1,2 @@
+| electron.js:8:16:8:78 | new Cli ... POST'}) | electron.js:31:16:31:22 | 'stuff' |
+| electron.js:8:16:8:78 | new Cli ... POST'}) | electron.js:32:14:32:25 | 'more stuff' |
--- a/javascript/ql/test/library-tests/frameworks/Electron/ClientRequest_getADataNode.ql
+++ b/javascript/ql/test/library-tests/frameworks/Electron/ClientRequest_getADataNode.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from Electron::ElectronClientRequest cr
+select cr, cr.getADataNode()
--- a/javascript/ql/test/library-tests/frameworks/NodeJSLib/ClientRequest_getADataNode.expected
+++ b/javascript/ql/test/library-tests/frameworks/NodeJSLib/ClientRequest_getADataNode.expected
@@ -0,0 +1,2 @@
+| src/http.js:27:16:27:73 | http.re ... POST'}) | src/http.js:50:16:50:22 | 'stuff' |
+| src/http.js:27:16:27:73 | http.re ... POST'}) | src/http.js:51:14:51:25 | 'more stuff' |
--- a/javascript/ql/test/library-tests/frameworks/NodeJSLib/ClientRequest_getADataNode.ql
+++ b/javascript/ql/test/library-tests/frameworks/NodeJSLib/ClientRequest_getADataNode.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from NodeJSLib::NodeJSClientRequest cr
+select cr, cr.getADataNode()