Implement sanitization of control characters in user input for hackerone (#32521)

GitOrigin-RevId: 859299da44b1c60220592c8f71a90536a5aa34a3

services/web/app/src/Features/Authentication/AuthenticationManager.mjs

@@ -3,6 +3,7 @@ import { User } from '../../models/User.mjs'
 import { db, ObjectId } from '../../infrastructure/mongodb.mjs'
 import bcrypt from 'bcrypt'
 import EmailHelper from '../Helpers/EmailHelper.mjs'
+import { sanitizeControlCharacters } from '../../infrastructure/Sanitize.mjs'
 
 import {
   InvalidEmailError,
@@ -114,6 +115,9 @@ const AuthenticationManager = {
   },
 
   async authenticate(query, password, auditLog, { enforceHIBPCheck = true }) {
+    if (typeof password === 'string') {
+      password = sanitizeControlCharacters(password)
+    }
     const { user, match } = await AuthenticationManager._checkUserPassword(
       query,
       password
@@ -312,6 +316,7 @@ const AuthenticationManager = {
     if (!user || !user.email || !user._id) {
       throw new Error('invalid user object')
     }
+    password = sanitizeControlCharacters(password)
     const validationError = this.validatePassword(password, user.email)
     if (validationError) {
       throw validationError

services/web/app/src/Features/Project/ProjectDetailsHandler.mjs

@@ -8,6 +8,7 @@ import PublicAccessLevels from '../Authorization/PublicAccessLevels.mjs'
 import Errors from '../Errors/Errors.js'
 import TokenGenerator from '../TokenGenerator/TokenGenerator.mjs'
 import ProjectHelper from './ProjectHelper.mjs'
+import { sanitizeControlCharacters } from '../../infrastructure/Sanitize.mjs'
 import settings from '@overleaf/settings'
 import { callbackify } from 'node:util'
 
@@ -121,6 +122,9 @@ async function renameProject(projectId, newName) {
 }
 
 async function validateProjectName(name) {
+  if (name != null) {
+    name = sanitizeControlCharacters(name)
+  }
   if (name == null || name.length === 0) {
     throw new Errors.InvalidNameError('Project name cannot be blank')
   }

services/web/app/src/Features/User/UserController.mjs

@@ -18,6 +18,7 @@ import EmailHandler from '../Email/EmailHandler.mjs'
 import UrlHelper from '../Helpers/UrlHelper.mjs'
 import { promisify } from 'node:util'
 import { expressify } from '@overleaf/promise-utils'
+import { sanitizeControlCharacters } from '../../infrastructure/Sanitize.mjs'
 import { acceptsJson } from '../../infrastructure/RequestContentTypeDetection.mjs'
 import Modules from '../../infrastructure/Modules.mjs'
 import OneTimeTokenHandler from '../Security/OneTimeTokenHandler.mjs'
@@ -361,17 +362,17 @@ async function updateUserSettings(req, res, next) {
     throw new OError('problem updating user settings', { userId })
   }
 
-  if (body.first_name != null) {
-    user.first_name = body.first_name.trim()
+  if (typeof body.first_name === 'string') {
+    user.first_name = sanitizeControlCharacters(body.first_name).trim()
   }
-  if (body.last_name != null) {
-    user.last_name = body.last_name.trim()
+  if (typeof body.last_name === 'string') {
+    user.last_name = sanitizeControlCharacters(body.last_name).trim()
   }
-  if (body.role != null) {
-    user.role = body.role.trim()
+  if (typeof body.role === 'string') {
+    user.role = sanitizeControlCharacters(body.role).trim()
   }
-  if (body.institution != null) {
-    user.institution = body.institution.trim()
+  if (typeof body.institution === 'string') {
+    user.institution = sanitizeControlCharacters(body.institution).trim()
   }
   if (body.mode != null) {
     user.ace.mode = body.mode

services/web/app/src/infrastructure/Sanitize.mjs

@@ -0,0 +1,22 @@
+// @ts-check
+
+/* eslint-disable no-control-regex, no-misleading-character-class */
+const CONTROL_CHARS_RE =
+  /[\u0000-\u001F\u007F-\u009F\u200B\u200C\u200D\u2060\uFEFF]/g
+/* eslint-enable no-control-regex, no-misleading-character-class */
+
+/**
+ * @param {string} value
+ * @returns {string}
+ */
+export function sanitizeControlCharacters(value) {
+  if (typeof value !== 'string') {
+    throw new TypeError(
+      `sanitizeControlCharacters expected a string, received ${typeof value}`
+    )
+  }
+  return value.replace(CONTROL_CHARS_RE, char => {
+    const code = /** @type {number} */ (char.codePointAt(0))
+    return `\\u${code.toString(16).padStart(4, '0')}`
+  })
+}

services/web/test/unit/src/Authentication/AuthenticationManager.test.mjs

@@ -148,6 +148,30 @@ describe('AuthenticationManager', function () {
         })
       })
 
+      describe('when the password contains control characters', function () {
+        it('should escape control characters before comparing', async function (ctx) {
+          const { user } =
+            await ctx.AuthenticationManager.promises.authenticate(
+              { email: ctx.email },
+              'test\u0000password',
+              null,
+              { enforceHIBPCheck: false }
+            )
+          expect(user).to.equal(null)
+        })
+
+        it('should escape zero-width spaces before comparing', async function (ctx) {
+          const { user } =
+            await ctx.AuthenticationManager.promises.authenticate(
+              { email: ctx.email },
+              'test\u200bpassword',
+              null,
+              { enforceHIBPCheck: false }
+            )
+          expect(user).to.equal(null)
+        })
+      })
+
       describe('when the encrypted passwords do not match', function () {
         beforeEach(async function (ctx) {
           ;({ user: ctx.result } =
@@ -251,6 +275,42 @@ describe('AuthenticationManager', function () {
         expect(hashedPassword.length).to.equal(60)
         expect(hashedPassword).to.match(/^\$2a\$04\$[a-zA-Z0-9/.]{53}$/)
       })
+
+      it('should escape control characters before hashing so login matches', async function (ctx) {
+        ctx.settings.passwordStrengthOptions = { allowAnyChars: true }
+        await ctx.AuthenticationManager.promises.setUserPasswordInV2(
+          ctx.user,
+          'test\u0000password'
+        )
+        const { hashedPassword } = ctx.db.users.updateOne.lastCall.args[1].$set
+
+        const matchEscaped = await bcrypt.compare(
+          'test\\u0000password',
+          hashedPassword
+        )
+        expect(matchEscaped).to.equal(true)
+
+        const matchRaw = await bcrypt.compare(
+          'test\u0000password',
+          hashedPassword
+        )
+        expect(matchRaw).to.equal(false)
+      })
+
+      it('should escape zero-width characters before hashing', async function (ctx) {
+        ctx.settings.passwordStrengthOptions = { allowAnyChars: true }
+        await ctx.AuthenticationManager.promises.setUserPasswordInV2(
+          ctx.user,
+          'test\u200bpassword'
+        )
+        const { hashedPassword } = ctx.db.users.updateOne.lastCall.args[1].$set
+
+        const matchEscaped = await bcrypt.compare(
+          'test\\u200bpassword',
+          hashedPassword
+        )
+        expect(matchEscaped).to.equal(true)
+      })
     })
   })