JS: Add test and dont check predecessors

Author: asgerf

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

@@ -109,7 +109,7 @@ where
   // the handler wouldn't work. However, if we can't find the cookie middleware, it
   // indicates that our middleware model is too incomplete, so in that case we
   // don't trust it to detect the presence of CSRF middleware either.
-  getARouteUsingCookies().flowsToExpr(handler.getPreviousMiddleware*()) and
+  getARouteUsingCookies().flowsToExpr(handler) and
   hasCookieMiddleware(handler, cookie) and
 
   // Only flag the first cookie parser registered first.

javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected

@@ -1,5 +1,6 @@
-| MissingCsrfMiddlewareBad.js:7:9:7:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:10:26:11:1 | functio ... es) {\\n} | here |
-| csurf_api_example.js:39:37:39:50 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_api_example.js:39:53:41:3 | functio ... e')\\n  } | here |
-| csurf_example.js:18:9:18:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_example.js:29:40:31:1 | functio ... sed')\\n} | here |
-| lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:23:42:25:1 | functio ... sed')\\n} | here |
-| lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:27:40:29:1 | functio ... sed')\\n} | here |
+| MissingCsrfMiddlewareBad.js:7:9:7:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:10:26:12:1 | functio ... il"];\\n} | here |
+| csurf_api_example.js:42:37:42:50 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_api_example.js:42:53:45:3 | functio ... e')\\n  } | here |
+| csurf_example.js:18:9:18:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_example.js:31:40:34:1 | functio ... sed')\\n} | here |
+| lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:26:42:29:1 | functio ... sed')\\n} | here |
+| lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:31:40:34:1 | functio ... sed')\\n} | here |
+| unused_cookies.js:6:9:6:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | unused_cookies.js:8:34:13:1 | (req, r ... Ok');\\n} | here |

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

@@ -8,4 +8,5 @@ app.use(cookieParser())
 app.use(passport.authorize({ session: true }))
 
 app.post('/changeEmail', function (req, res) {
+    let newEmail = req.cookies["newEmail"];
 })

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

@@ -10,4 +10,5 @@ app.use(passport.authorize({ session: true }))
 app.use(csrf({ cookie:true }))
 
 app.post('/changeEmail', function (req, res) {
+    let newEmail = req.cookies["newEmail"];
 })

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

@@ -21,22 +21,26 @@ app.use(cookieParser())
 app.use(csrf({ cookie: true }))
 
 app.get('/form', function (req, res) {
+  let newEmail = req.cookies["newEmail"];
   // pass the csrfToken to the view
   res.render('send', { csrfToken: req.csrfToken() })
 })
 
 app.post('/process', function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   res.send('csrf was required to get here')
 })
 
 function createApiRouter () {
   var router = new express.Router()
 
   router.post('/getProfile', function (req, res) { // OK - cookies are not parsed
+    let newEmail = req.cookies["newEmail"];
     res.send('no csrf to get here')
   })
 
   router.post('/getProfile_unsafe', cookieParser(), function (req, res) { // NOT OK - may use cookies
+    let newEmail = req.cookies["newEmail"];
     res.send('no csrf to get here')
   })
 

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

@@ -18,14 +18,17 @@ var app = express()
 app.use(cookieParser())
 
 app.get('/form', csrfProtection, function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   // pass the csrfToken to the view
   res.render('send', { csrfToken: req.csrfToken() })
 })
 
 app.post('/process', parseForm, csrfProtection, function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })
 
 app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })

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

@@ -9,21 +9,26 @@ var app = express()
 app.use(cookieParser())
 
 app.post('/process', parseForm, lusca.csrf(), function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })
 
 app.post('/process', parseForm, lusca({csrf:true}), function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })
 
 app.post('/process', parseForm, lusca({csrf:{}}), function (req, res) { // OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })
 
 app.post('/process', parseForm, lusca(), function (req, res) { // NOT OK - missing csrf option
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })
 
 app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK
+  let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
 })

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

@@ -0,0 +1,20 @@
+let express = require('express');
+let cookieParser = require('cookie-parser');
+
+let app = express();
+
+app.use(cookieParser());
+
+app.post('/doSomethingTerrible', (req, res) => { // NOT OK - uses cookies
+    if (req.cookies['secret'] === app.secret) {
+        somethingTerrible();
+    }
+    res.end('Ok');
+});
+
+app.post('/doSomethingElse', (req, res) => { // OK - doesn't actually use cookies
+    somethingElse(req.query['data']);
+    res.end('Ok');
+});
+
+app.listen();