JS: Ignore calls and csrf/captcha access

Author: asgerf

javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql

@@ -12,11 +12,22 @@
 
 import javascript
 
+/** Gets the string `session` or `cookies`, the parts of `req` containing cookie data. */
+string sessionOrCookies() {
+  result = "session" or result = "cookies"
+}
+
 /** Gets a data flow node that flows to the base of an access to `cookies` or `session`. */
 private DataFlow::SourceNode nodeLeadingToCookieAccess(DataFlow::TypeBackTracker t) {
   t.start() and
-  exists(string name | name = "session" or name = "cookies" |
-    exists(result.getAPropertyRead(name))
+  exists(DataFlow::PropRead value |
+    value = result.getAPropertyRead(sessionOrCookies()).getAPropertyRead() and
+
+    // Ignore accesses to values that are part of a CSRF or captcha check
+    not value.getPropertyName().regexpMatch("(?i).*(csrf|xsrf|captcha).*") and
+
+    // Ignore calls like `req.session.save()`
+    not value = any(DataFlow::InvokeNode call).getCalleeNode()
   )
   or
   exists(DataFlow::TypeBackTracker t2 |

javascript/ql/test/query-tests/Security/CWE-352/unused_cookies.js

@@ -17,4 +17,13 @@ app.post('/doSomethingElse', (req, res) => { // OK - doesn't actually use cookie
     res.end('Ok');
 });
 
+app.post('/doWithCaptcha', (req, res) => { // OK - attacker can't guess the captcha value either
+    if (req.session['captcha'] !== req.query['captcha']) {
+        res.end("You guessed wrong, that 'u' was actually a 'U'. Try again.");
+        return;
+    }
+    somethingElse(req.query['data']);
+    res.end('Ok');
+});
+
 app.listen();