diff --git a/services/web/app/src/Features/Authentication/AuthenticationManager.mjs b/services/web/app/src/Features/Authentication/AuthenticationManager.mjs index e49c5be410..bcd9fa9108 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationManager.mjs +++ b/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 diff --git a/services/web/app/src/Features/Project/ProjectDetailsHandler.mjs b/services/web/app/src/Features/Project/ProjectDetailsHandler.mjs index 7ef920c087..a5feaa2cdf 100644 --- a/services/web/app/src/Features/Project/ProjectDetailsHandler.mjs +++ b/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') } diff --git a/services/web/app/src/Features/User/UserController.mjs b/services/web/app/src/Features/User/UserController.mjs index ae2d116827..73f7ed4c06 100644 --- a/services/web/app/src/Features/User/UserController.mjs +++ b/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 diff --git a/services/web/app/src/infrastructure/Sanitize.mjs b/services/web/app/src/infrastructure/Sanitize.mjs new file mode 100644 index 0000000000..e5fc27489d --- /dev/null +++ b/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')}` + }) +} diff --git a/services/web/test/unit/src/Authentication/AuthenticationManager.test.mjs b/services/web/test/unit/src/Authentication/AuthenticationManager.test.mjs index c56112835b..7e76c7da41 100644 --- a/services/web/test/unit/src/Authentication/AuthenticationManager.test.mjs +++ b/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) + }) }) })