Merge pull request github#2982 from esbena/js/request-model-with-chaining

Approved by asgerf

Author: semmle-qlci

change-notes/1.24/analysis-javascript.md

@@ -32,6 +32,7 @@
   - [http2](https://nodejs.org/api/http2.html)
   - [lazy-cache](https://www.npmjs.com/package/lazy-cache)
   - [react](https://www.npmjs.com/package/react)
+  - [request](https://www.npmjs.com/package/request)
   - [send](https://www.npmjs.com/package/send)
   - [typeahead.js](https://www.npmjs.com/package/typeahead.js)
   - [ws](https://github.com/websockets/ws)

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

@@ -110,29 +110,37 @@ module ClientRequest {
    */
   private string httpMethodName() { result = any(HTTP::RequestMethodName m).toLowerCase() }
 
+  /**
+   * Gets a model of an instance of the `request` library, or one of
+   * its wrappers, `promise` is true if the instance uses promises
+   * rather than callbacks.
+   */
+  private DataFlow::SourceNode getRequestLibrary(boolean promise) {
+    exists(string moduleName | result = DataFlow::moduleImport(moduleName) |
+      promise = false and
+      moduleName = "request"
+      or
+      promise = true and
+      (
+        moduleName = "request-promise" or
+        moduleName = "request-promise-any" or
+        moduleName = "request-promise-native"
+      )
+    )
+    or
+    result = getRequestLibrary(promise).getAMethodCall("defaults")
+  }
+
   /**
    * A model of a URL request made using the `request` library.
    */
   class RequestUrlRequest extends ClientRequest::Range, DataFlow::CallNode {
     boolean promise;
 
     RequestUrlRequest() {
-      exists(string moduleName, DataFlow::SourceNode callee | this = callee.getACall() |
-        (
-          promise = false and
-          moduleName = "request"
-          or
-          promise = true and
-          (
-            moduleName = "request-promise" or
-            moduleName = "request-promise-any" or
-            moduleName = "request-promise-native"
-          )
-        ) and
-        (
-          callee = DataFlow::moduleImport(moduleName) or
-          callee = DataFlow::moduleMember(moduleName, httpMethodName())
-        )
+      exists(DataFlow::SourceNode callee | this = callee.getACall() |
+        callee = getRequestLibrary(promise) or
+        callee = getRequestLibrary(promise).getAPropertyRead(httpMethodName())
       )
     }
 

javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequests.expected

@@ -52,6 +52,9 @@ test_ClientRequest
 | tst.js:147:5:147:19 | got.stream(url) |
 | tst.js:151:5:151:23 | superagent.get(url) |
 | tst.js:160:5:160:17 | xhr.send(url) |
+| tst.js:171:2:171:10 | base(url) |
+| tst.js:172:2:172:14 | variant1(url) |
+| tst.js:173:2:173:14 | variant2(url) |
 test_getADataNode
 | 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 |
@@ -144,6 +147,9 @@ test_getUrl
 | tst.js:147:5:147:19 | got.stream(url) | tst.js:147:16:147:18 | url |
 | tst.js:151:5:151:23 | superagent.get(url) | tst.js:151:20:151:22 | url |
 | tst.js:160:5:160:17 | xhr.send(url) | tst.js:160:14:160:16 | url |
+| tst.js:171:2:171:10 | base(url) | tst.js:171:7:171:9 | url |
+| tst.js:172:2:172:14 | variant1(url) | tst.js:172:11:172:13 | url |
+| tst.js:173:2:173:14 | variant2(url) | tst.js:173:11:173:13 | url |
 test_getAResponseDataNode
 | tst.js:19:5:19:23 | requestPromise(url) | tst.js:19:5:19:23 | requestPromise(url) | text | true |
 | tst.js:21:5:21:23 | superagent.get(url) | tst.js:21:5:21:23 | superagent.get(url) | stream | true |

javascript/ql/test/library-tests/frameworks/ClientRequests/tst.js

@@ -163,3 +163,12 @@ import {ClientRequest, net} from 'electron';
         xhr.getResponseHeaders();
     });
 })
+
+(function() {
+	let base = request;
+	let variant1 = base.defaults({});
+	let variant2 = variant1.defaults({});
+	base(url);
+	variant1(url);
+	variant2(url);
+});