From 21fe0de46dea9b4fb879429308ca45773f368a7d Mon Sep 17 00:00:00 2001
From: Jack Foltz <jack@foltz.io>
Date: Wed, 1 Aug 2018 12:34:15 -0400
Subject: [PATCH] Extend body verifier to sanitize, regex trim, and length
 check

---
 app/routes/api/auth.js | 15 +++++++--------
 app/util/verifyBody.js |  9 +++++++++
 test/api.js            |  6 +++---
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/app/routes/api/auth.js b/app/routes/api/auth.js
index cda9d28..c0db9f7 100644
--- a/app/routes/api/auth.js
+++ b/app/routes/api/auth.js
@@ -50,13 +50,6 @@ const validateInvite = wrap(async (req, res, next) => {
 const validateUsername = wrap(async (req, res, next) => {
     const username = req.body.username;
 
-    if (username.length > config.get('User.Username.maxLength'))
-        return res.status(422).json({message: 'Username too long.'});
-
-    const restrictedRegex = new RegExp(config.get('User.Username.restrictedChars'), 'g');
-    if (username !== req.sanitize(username).replace(restrictedRegex, ''))
-        return res.status(422).json({message: 'Username contains invalid characters.'});
-
     const count = await User.countDocuments({username: username}).catch(next);
     if (count !== 0)
         return res.status(422).json({message: 'Username in use.'});
@@ -65,7 +58,13 @@ const validateUsername = wrap(async (req, res, next) => {
 });
 
 const registerProps = [
-    {name: 'displayname', type: 'string'},
+    {
+        name: 'displayname',
+        type: 'string',
+        maxLength: config.get('User.Username.maxLength'),
+        sanitize: true,
+        restrict: new RegExp(config.get('User.Username.restrictedChars')),
+    },
     {name: 'password', type: 'string'},
     {name: 'invite', type: 'string'}];
 router.post('/register',
diff --git a/app/util/verifyBody.js b/app/util/verifyBody.js
index 08674f2..97a7f1e 100644
--- a/app/util/verifyBody.js
+++ b/app/util/verifyBody.js
@@ -16,6 +16,15 @@ const verifyBody = expectedProps =>
 
             if (prop && expected.instance && !(prop instanceof expected.instance))
                 return res.status(400).json({message: expected.name + ' malformed.'});
+
+            if (prop && expected.maxLength && prop.length > expected.maxLength)
+                return res.status(422).json({message: expected.name + ' too long.'});
+
+            if (prop && expected.sanitize && req.sanitize(prop) !== prop)
+                return res.status(422).json({message: expected.name + ' contains invalid characters.'});
+
+            if (prop && expected.restrict && prop.replace(expected.restrict, '') !== prop)
+                return res.status(422).json({message: expected.name + ' contains invalid characters.'});
         }
         next();
     };
diff --git a/test/api.js b/test/api.js
index 278a1ad..592d65b 100644
--- a/test/api.js
+++ b/test/api.js
@@ -121,20 +121,20 @@ describe('Authentication', function() {
                     {displayname: 'user name', password: 'pass', invite: 'code2'}
                 ];
 
-                const failMsg = 'Username contains invalid characters.';
+                const failMsg = 'displayname contains invalid characters.';
                 return Promise.all(users.map(user => verifyRejectedUsername(user, failMsg)));
             });
 
             it('MUST NOT register a username containing HTML', async () => {
                 await util.createTestInvite();
                 const user = {displayname: 'user<svg/onload=alert("XSS")>', password: 'pass', invite: 'code'};
-                return verifyRejectedUsername(user, 'Username contains invalid characters.');
+                return verifyRejectedUsername(user, 'displayname contains invalid characters.');
             });
 
             it('MUST NOT register a username with too many characters', async () => {
                 await util.createTestInvite();
                 const user = {displayname: '123456789_123456789_123456789_1234567', password: 'pass', invite: 'code'};
-                return verifyRejectedUsername(user, 'Username too long.');
+                return verifyRejectedUsername(user, 'displayname too long.');
             })
         });