JS: Use post-processed inline test in MissingCsrfMiddleware

This query flags the cookie-parsing middleware in order to consolidate huge numbers of alerts into a single alert, which is more manageable. But simply annotating the cookie-parsing middleware with 'Alert' isn't a very useful, we want to annotate which middlewares are vulnerable.

Author: asgerf

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

@@ -1 +1,2 @@
-Security/CWE-352/MissingCsrfMiddleware.ql
+query: Security/CWE-352/MissingCsrfMiddleware.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

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

@@ -4,17 +4,17 @@ var passport = require('passport');
 
 var app = express();
 
-app.use(cookieParser());
+app.use(cookieParser()); // $ Alert
 app.use(passport.authorize({ session: true }));
 
 app.post('/changeEmail', function (req, res) {
     let newEmail = req.cookies["newEmail"];
-});
+}); // $ RelatedLocation
 
 (function () {
     var app = express();
 
-    app.use(cookieParser());
+    app.use(cookieParser()); // $ Alert
     app.use(passport.authorize({ session: true }));
 
     const errorCatch = (fn) =>
@@ -24,13 +24,13 @@ app.post('/changeEmail', function (req, res) {
 
     app.post('/changeEmail', errorCatch(async function (req, res) {
         let newEmail = req.cookies["newEmail"];
-    }));
+    })); // $ RelatedLocation
 })
 
 (function () {
     var app = express();
 
-    app.use(cookieParser());
+    app.use(cookieParser()); // $ Alert
     app.use(passport.authorize({ session: true }));
 
     const errorCatch = (fn) =>
@@ -40,9 +40,9 @@ app.post('/changeEmail', function (req, res) {
 
     app.post('/changeEmail', errorCatch(async function (req, res) {
         let newEmail = req.cookies["newEmail"];
-    }));
+    })); // $ RelatedLocation
 
     app.post('/doLoginStuff', errorCatch(async function (req, res) {
         req.session.user = loginStuff(req);
-    }));
+    })); // $ RelatedLocation
 })

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

@@ -39,10 +39,10 @@ function createApiRouter () {
     res.send('no csrf to get here')
   })
 
-  router.post('/getProfile_unsafe', cookieParser(), function (req, res) { // NOT OK - may use cookies
+  router.post('/getProfile_unsafe', cookieParser(), function (req, res) { // $ Alert - may use cookies
     let newEmail = req.cookies["newEmail"];
     res.send('no csrf to get here')
-  })
+  }) // $ RelatedLocation
 
   return router
 }

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

@@ -15,7 +15,7 @@ var app = express()
 
 // parse cookies
 // we need this because "cookie" is true in csrfProtection
-app.use(cookieParser())
+app.use(cookieParser()) // $ Alert
 
 app.get('/form', csrfProtection, function (req, res) { // OK
   let newEmail = req.cookies["newEmail"];
@@ -28,7 +28,7 @@ app.post('/process', parseForm, csrfProtection, function (req, res) { // OK
   res.send('data is being processed')
 })
 
-app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK
+app.post('/process_unsafe', parseForm, function (req, res) {
   let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
-})
+}) // $ RelatedLocation

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

@@ -2,7 +2,7 @@ const fastify = require('fastify')
 
 const app = fastify();
 
-app.register(require('fastify-cookie'));
+app.register(require('fastify-cookie')); // $ Alert
 app.register(require('fastify-csrf'));
 
 app.route({
@@ -17,10 +17,10 @@ app.route({
 app.route({
   method: 'POST',
   path: '/',
-  handler: async (req, reply) => { // NOT OK - lacks CSRF protection
+  handler: async (req, reply) => { // lacks CSRF protection
     req.session.blah;
     return req.body
-  }
+  } // $ RelatedLocation
 })
 
 

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

@@ -4,7 +4,7 @@ const fp = require('fastify-plugin');
 const app = fastify();
 
 function plugin(app) {
-  app.register(require('fastify-cookie'));
+  app.register(require('fastify-cookie')); // $ Alert
   app.register(require('fastify-csrf'));
 }
 app.register(fp(plugin));
@@ -21,10 +21,10 @@ app.route({
 app.route({
   method: 'POST',
   path: '/',
-  handler: async (req, reply) => { // NOT OK - lacks CSRF protection
+  handler: async (req, reply) => { // lacks CSRF protection
     req.session.blah;
     return req.body
-  }
+  } // $ RelatedLocation
 })
 
 

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

@@ -6,7 +6,7 @@ var parseForm = bodyParser.urlencoded({ extended: false })
 var lusca = require('lusca');
 
 var app = express()
-app.use(cookieParser())
+app.use(cookieParser()) // $ Alert
 
 app.post('/process', parseForm, lusca.csrf(), function (req, res) { // OK
   let newEmail = req.cookies["newEmail"];
@@ -23,12 +23,12 @@ app.post('/process', parseForm, lusca({csrf:{}}), function (req, res) { // OK
   res.send('data is being processed')
 })
 
-app.post('/process', parseForm, lusca(), function (req, res) { // NOT OK - missing csrf option
+app.post('/process', parseForm, lusca(), function (req, res) { // missing csrf option
   let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
-})
+}) // $ RelatedLocation
 
-app.post('/process_unsafe', parseForm, function (req, res) { // NOT OK
+app.post('/process_unsafe', parseForm, function (req, res) {
   let newEmail = req.cookies["newEmail"];
   res.send('data is being processed')
-})
+}) // $ RelatedLocation

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

@@ -3,11 +3,11 @@ const cookieParser = require('cookie-parser')
 const csrf = require('csurf')
 
 const app = express()
-app.use(cookieParser())
+app.use(cookieParser()) // $ Alert
 
-app.post('/unsafe', (req, res) => { // NOT OK
+app.post('/unsafe', (req, res) => {
   req.cookies.x;
-});
+}); // $ RelatedLocation
 
 function middlewares() {
   return express.Router()

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

@@ -3,14 +3,14 @@ let cookieParser = require('cookie-parser');
 
 let app = express();
 
-app.use(cookieParser());
+app.use(cookieParser()); // $ Alert
 
-app.post('/doSomethingTerrible', (req, res) => { // NOT OK - uses cookies
+app.post('/doSomethingTerrible', (req, res) => { // uses cookies
     if (req.cookies['secret'] === app.secret) {
         somethingTerrible();
     }
     res.end('Ok');
-});
+}); // $ RelatedLocation
 
 app.post('/doSomethingElse', (req, res) => { // OK - doesn't actually use cookies
     somethingElse(req.query['data']);
@@ -26,14 +26,14 @@ app.post('/doWithCaptcha', (req, res) => { // OK - attacker can't guess the capt
     res.end('Ok');
 });
 
-app.post('/user', (req, res) => { // NOT OK - access to req.user is unprotected
+app.post('/user', (req, res) => { // access to req.user is unprotected
     somethingElse(req.user.name);
     res.end('Ok');
-});
+}); // $ RelatedLocation
 
-app.post('/session', (req, res) => { // NOT OK - access to req.session is unprotected
+app.post('/session', (req, res) => { // access to req.session is unprotected
     somethingElse(req.session.name);
     res.end('Ok');
-});
+}); // $ RelatedLocation
 
 app.listen();