Merge pull request #444 from esben-semmle/js/browser-based-client-requests

JS: add models of $.ajax, $.getJSON and XMLHttpRequst

Author: Max Schaefer

change-notes/1.19/analysis-javascript.md

@@ -30,19 +30,20 @@
 
 | **Query**                      | **Expected impact**        | **Change**                                   |
 |--------------------------------|----------------------------|----------------------------------------------|
-| Useless assignment to local variable | Fewer false-positive results | This rule now recognizes additional ways default values can be set. |
+| Client side cross-site scripting | More results | This rule now also flags HTML injection in the body of an email. |
+| Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. |
+| Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
 | Regular expression injection | Fewer false-positive results | This rule now identifies calls to `String.prototype.search` with more precision. |
-| Unbound event handler receiver | Fewer false-positive results | This rule now recognizes additional ways class methods can be bound. |
 | Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
-| Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
+| Self assignment | Fewer false-positive results | This rule now ignores self-assignments preceded by a JSDoc comment with a `@type` tag. |
 | Server-side URL redirect | More results | This rule now recognizes redirection calls in more cases. |
+| Unbound event handler receiver | Fewer false-positive results | This rule now recognizes additional ways class methods can be bound. |
+| Uncontrolled data used in remote request | More results | This rule now recognizes additional kinds of requests. |
+| Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. |
 | Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that may be used by `eval` calls. |
 | Unused variable, import, function or class | Fewer results | This rule now flags import statements with multiple unused imports once. |
+| Useless assignment to local variable | Fewer false-positive results | This rule now recognizes additional ways default values can be set. |
 | Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
-| Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. |
-| Self assignment | Fewer false-positive results | This rule now ignores self-assignments preceded by a JSDoc comment with a `@type` tag. |
-| Client side cross-site scripting | More results | This rule now also flags HTML injection in the body of an email. |
-| Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. |
 
 ## Changes to QL libraries
 

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

@@ -246,3 +246,14 @@ private class SuperAgentUrlRequest extends CustomClientRequest {
   }
 
 }
+
+/**
+ * A model of a URL request made using the `XMLHttpRequest` browser class.
+ */
+private class XMLHttpRequest extends CustomClientRequest {
+  XMLHttpRequest() { this = DataFlow::globalVarRef("XMLHttpRequest").getAnInstantiation() }
+
+  override DataFlow::Node getUrl() { result = getAMethodCall("open").getArgument(1) }
+
+  override DataFlow::Node getADataNode() { result = getAMethodCall("send").getArgument(0) }
+}

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

@@ -340,3 +340,24 @@ private class JQueryChainedElement extends DOM::Element {
     )
   }
 }
+
+/**
+ * A model of a URL request made using the `jQuery.ajax` or `jQuery.getJSON`.
+ */
+private class JQueryClientRequest extends CustomClientRequest {
+  JQueryClientRequest() {
+    exists(string name |
+      name = "ajax" or
+      name = "getJSON"
+      |
+      this = jquery().getAMemberCall(name)
+    )
+  }
+
+  override DataFlow::Node getUrl() {
+    result = getArgument(0) or
+    result = getOptionArgument([0 .. 1], "url")
+  }
+
+  override DataFlow::Node getADataNode() { result = getOptionArgument([0 .. 1], "data") }
+}

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

@@ -27,3 +27,8 @@
 | 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) |
+| tst.js:74:5:74:29 | $.ajax( ...  data}) |
+| tst.js:75:5:75:35 | $.ajax( ...  data}) |
+| tst.js:77:5:77:32 | $.getJS ...  data}) |
+| tst.js:78:5:78:38 | $.getJS ...  data}) |
+| tst.js:80:15:80:34 | new XMLHttpRequest() |

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

@@ -15,3 +15,6 @@
 | 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 |
+| tst.js:74:5:74:29 | $.ajax( ...  data}) | tst.js:74:24:74:27 | data |
+| tst.js:77:5:77:32 | $.getJS ...  data}) | tst.js:77:27:77:30 | data |
+| tst.js:80:15:80:34 | new XMLHttpRequest() | tst.js:82:14:82:17 | data |

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

@@ -31,3 +31,10 @@
 | 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 |
+| tst.js:74:5:74:29 | $.ajax( ...  data}) | tst.js:74:12:74:14 | url |
+| tst.js:75:5:75:35 | $.ajax( ...  data}) | tst.js:75:12:75:34 | {url: u ... : data} |
+| tst.js:75:5:75:35 | $.ajax( ...  data}) | tst.js:75:18:75:20 | url |
+| tst.js:77:5:77:32 | $.getJS ...  data}) | tst.js:77:15:77:17 | url |
+| tst.js:78:5:78:38 | $.getJS ...  data}) | tst.js:78:15:78:37 | {url: u ... : data} |
+| tst.js:78:5:78:38 | $.getJS ...  data}) | tst.js:78:21:78:23 | url |
+| tst.js:80:15:80:34 | new XMLHttpRequest() | tst.js:81:17:81:19 | url |

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

@@ -69,3 +69,15 @@ import {ClientRequest, net} from 'electron';
     superagent.get(url).unknown(nonData).query(queryData);
 
 });
+
+(function() {
+    $.ajax(url, {data: data});
+    $.ajax({url: url, tdata: data});
+
+    $.getJSON(url, {data: data});
+    $.getJSON({url: url, tdata: data});
+
+    var xhr = new XMLHttpRequest();
+    xhr.open(_, url);
+    xhr.send(data);
+});