Fix issue with CSRF black/whitelists and routes containing optional p…

…arams when the requested URL contains a querystring but NOT the optional param (whew!) Also add tests for the above case and others involving querystrings.
balderdashy · Jan 26, 2018 · b3afed7 · b3afed7
1 parent 663527f
commit b3afed7
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 4 deletions.
diff --git a/lib/hooks/security/csrf/index.js b/lib/hooks/security/csrf/index.js
@@ -87,7 +87,7 @@ module.exports = function(sails) {
         if (sails.config.security.csrf === false) {
           // If nothing in the whitelist matches, continue on without checking for a CSRF token.
           if (!_.any(whitelist, function(whitelistedRoute) {
-            return req.url.match(whitelistedRoute.regex) && (!whitelistedRoute.method || whitelistedRoute.method === req.method.toLowerCase());
+            return req.path.match(whitelistedRoute.regex) && (!whitelistedRoute.method || whitelistedRoute.method === req.method.toLowerCase());
           })) {
             return next();
           }
@@ -96,7 +96,7 @@ module.exports = function(sails) {
         else {
           // If anything in the blacklist matches, continue on without checking for a CSRF token.
           if (_.any(blacklist, function(blacklistedRoute) {
-            return req.url.match(blacklistedRoute.regex) && (!blacklistedRoute.method || blacklistedRoute.method === req.method.toLowerCase());
+            return req.path.match(blacklistedRoute.regex) && (!blacklistedRoute.method || blacklistedRoute.method === req.method.toLowerCase());
           })) {
             return next();
           }

diff --git a/test/integration/hook.csrf.test.js b/test/integration/hook.csrf.test.js
@@ -168,7 +168,7 @@ describe('CSRF ::', function() {
       });
     });
 
-    describe('with CSRF set to true, blacklisting \'POST /foo/:id, /user\'}', function() {
+    describe('with CSRF set to true, blacklisting \'POST /foo/:id, POST /bar/:id?, /user\'}', function() {
 
       before(function() {
 
@@ -177,7 +177,9 @@ describe('CSRF ::', function() {
             csrf: true
           },
           routes: {
+            // Note -- since we don't actually define a target for these, requests that pass CSRF should return a 404.
             'POST /foo/:id': {csrf: false},
+            '/bar/:id?': {csrf: false},
             '/user': {csrf: false}
           }
         };
@@ -202,7 +204,52 @@ describe('CSRF ::', function() {
           if (err && err.status === 404) {
             return done();
           }
-          done(new Error('Expected a 404 error, instead got: ' + err || response.body));
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
+        });
+      });
+
+      it('a POST request on /foo/12?abc=123 without a CSRF token should result in a 404 response', function(done) {
+        sailsApp.request({url: '/foo/12?abc=123', method: 'post'}, function(err, response) {
+          if (err && err.status === 404) {
+            return done();
+          }
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
+        });
+      });
+
+      it('a POST request on /bar/12 without a CSRF token should result in a 404 response', function(done) {
+        sailsApp.request({url: '/bar/12', method: 'post'}, function(err, response) {
+          if (err && err.status === 404) {
+            return done();
+          }
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
+        });
+      });
+
+      it('a POST request on /bar/12?abc=123 without a CSRF token should result in a 404 response', function(done) {
+        sailsApp.request({url: '/bar/12?abc=123', method: 'post'}, function(err, response) {
+          if (err && err.status === 404) {
+            return done();
+          }
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
+        });
+      });
+
+      it('a POST request on /bar without a CSRF token should result in a 404 response', function(done) {
+        sailsApp.request({url: '/bar', method: 'post'}, function(err, response) {
+          if (err && err.status === 404) {
+            return done();
+          }
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
+        });
+      });
+
+      it('a POST request on /bar?abc=123 without a CSRF token should result in a 404 response', function(done) {
+        sailsApp.request({url: '/bar?abc=123', method: 'post'}, function(err, response) {
+          if (err && err.status === 404) {
+            return done();
+          }
+          done(new Error('Expected a 404 error, instead got: ' + (err || response.body)));
         });
       });