From 2d2b2fa1fe8549b7def124425baf11eec2f373da Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 22 Mar 2026 00:15:52 +0000 Subject: [PATCH 1/3] fix: GHSA-2299-ghjr-6vjp v9 --- spec/vulnerabilities.spec.js | 75 +++++++++++++++++++ src/Adapters/Storage/Mongo/MongoTransform.js | 6 +- .../Postgres/PostgresStorageAdapter.js | 5 ++ src/Routers/UsersRouter.js | 28 +++++-- 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 8645dd524c..1c3ecaf23e 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -4238,6 +4238,81 @@ describe('(GHSA-g4cf-xj29-wqqr) DoS via unindexed database query for unconfigure }); expect(authDataQueries.length).toBeGreaterThan(0); }); + + describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => { + const mfaHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'Content-Type': 'application/json', + }; + + beforeEach(async () => { + await reconfigureServer({ + auth: { + mfa: { + enabled: true, + options: ['TOTP'], + algorithm: 'SHA1', + digits: 6, + period: 30, + }, + }, + }); + }); + + it('rejects concurrent logins using the same MFA recovery code', async () => { + const OTPAuth = require('otpauth'); + const user = await Parse.User.signUp('mfauser', 'password123'); + const secret = new OTPAuth.Secret(); + const totp = new OTPAuth.TOTP({ + algorithm: 'SHA1', + digits: 6, + period: 30, + secret, + }); + const token = totp.generate(); + await user.save( + { authData: { mfa: { secret: secret.base32, token } } }, + { sessionToken: user.getSessionToken() } + ); + + // Get recovery codes from stored auth data + await user.fetch({ useMasterKey: true }); + const recoveryCode = user.get('authData').mfa.recovery[0]; + expect(recoveryCode).toBeDefined(); + + // Send concurrent login requests with the same recovery code + const loginWithRecovery = () => + request({ + method: 'POST', + url: 'http://localhost:8378/1/login', + headers: mfaHeaders, + body: JSON.stringify({ + username: 'mfauser', + password: 'password123', + authData: { + mfa: { + token: recoveryCode, + }, + }, + }), + }); + + const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery())); + + const succeeded = results.filter(r => r.status === 'fulfilled'); + const failed = results.filter(r => r.status === 'rejected'); + + // Exactly one request should succeed; all others should fail + expect(succeeded.length).toBe(1); + expect(failed.length).toBe(9); + + // Verify the recovery code has been consumed + await user.fetch({ useMasterKey: true }); + const remainingRecovery = user.get('authData').mfa.recovery; + expect(remainingRecovery).not.toContain(recoveryCode); + }); + }); }); describe('(GHSA-p2w6-rmh7-w8q3) SQL Injection via aggregate and distinct field names in PostgreSQL adapter', () => { diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index bbaefdd55a..0fd13017b3 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -304,11 +304,11 @@ function transformQueryKeyValue(className, key, value, schema, count = false) { return { key: 'times_used', value: value }; default: { // Other auth data - const authDataMatch = key.match(/^authData\.([a-zA-Z0-9_]+)\.id$/); + const authDataMatch = key.match(/^authData\.([a-zA-Z0-9_]+)(\.(.+))?$/); if (authDataMatch && className === '_User') { const provider = authDataMatch[1]; - // Special-case auth data. - return { key: `_auth_data_${provider}.id`, value }; + const subField = authDataMatch[3]; + return { key: `_auth_data_${provider}${subField ? `.${subField}` : ''}`, value }; } } } diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 7a2bafc460..b99fd9f1d4 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -340,6 +340,11 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus patterns.push(`$${index}:raw = $${index + 1}::text`); values.push(name, fieldValue); index += 2; + } else if (typeof fieldValue === 'object') { + name = transformDotFieldToComponents(fieldName).join('->'); + patterns.push(`($${index}:raw)::jsonb = $${index + 1}::jsonb`); + values.push(name, JSON.stringify(fieldValue)); + index += 2; } } } else if (fieldValue === null || fieldValue === undefined) { diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index beaa08f6f2..fff8f3b3d0 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -286,12 +286,28 @@ export class UsersRouter extends ClassesRouter { // If we have some new validated authData update directly if (validatedAuthData && Object.keys(validatedAuthData).length) { - await req.config.database.update( - '_User', - { objectId: user.objectId }, - { authData: validatedAuthData }, - {} - ); + const query = { objectId: user.objectId }; + // Optimistic locking: include the original authData state in the WHERE clause + // for providers whose data is being updated. This prevents concurrent requests + // from both succeeding when consuming single-use tokens (e.g. MFA recovery codes). + if (user.authData) { + for (const provider of Object.keys(validatedAuthData)) { + const original = user.authData[provider]; + if (original && JSON.stringify(original) !== JSON.stringify(validatedAuthData[provider])) { + for (const [field, value] of Object.entries(original)) { + query[`authData.${provider}.${field}`] = value; + } + } + } + } + try { + await req.config.database.update('_User', query, { authData: validatedAuthData }, {}); + } catch (error) { + if (error.code === Parse.Error.OBJECT_NOT_FOUND) { + throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'Invalid auth data'); + } + throw error; + } } const { sessionData, createSession } = RestWrite.createSession(req.config, { From 4c42259167ff30ed61e4bad465f75e8fc5f4db31 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 22 Mar 2026 00:31:05 +0000 Subject: [PATCH 2/3] fix: address CodeRabbit review feedback --- spec/vulnerabilities.spec.js | 124 +++++++++--------- .../Postgres/PostgresStorageAdapter.js | 5 +- 2 files changed, 66 insertions(+), 63 deletions(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 1c3ecaf23e..a866689c24 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -4238,80 +4238,80 @@ describe('(GHSA-g4cf-xj29-wqqr) DoS via unindexed database query for unconfigure }); expect(authDataQueries.length).toBeGreaterThan(0); }); +}); - describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => { - const mfaHeaders = { - 'X-Parse-Application-Id': 'test', - 'X-Parse-REST-API-Key': 'rest', - 'Content-Type': 'application/json', - }; +describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => { + const mfaHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'Content-Type': 'application/json', + }; - beforeEach(async () => { - await reconfigureServer({ - auth: { - mfa: { - enabled: true, - options: ['TOTP'], - algorithm: 'SHA1', - digits: 6, - period: 30, - }, + beforeEach(async () => { + await reconfigureServer({ + auth: { + mfa: { + enabled: true, + options: ['TOTP'], + algorithm: 'SHA1', + digits: 6, + period: 30, }, - }); + }, }); + }); - it('rejects concurrent logins using the same MFA recovery code', async () => { - const OTPAuth = require('otpauth'); - const user = await Parse.User.signUp('mfauser', 'password123'); - const secret = new OTPAuth.Secret(); - const totp = new OTPAuth.TOTP({ - algorithm: 'SHA1', - digits: 6, - period: 30, - secret, - }); - const token = totp.generate(); - await user.save( - { authData: { mfa: { secret: secret.base32, token } } }, - { sessionToken: user.getSessionToken() } - ); + it('rejects concurrent logins using the same MFA recovery code', async () => { + const OTPAuth = require('otpauth'); + const user = await Parse.User.signUp('mfauser', 'password123'); + const secret = new OTPAuth.Secret(); + const totp = new OTPAuth.TOTP({ + algorithm: 'SHA1', + digits: 6, + period: 30, + secret, + }); + const token = totp.generate(); + await user.save( + { authData: { mfa: { secret: secret.base32, token } } }, + { sessionToken: user.getSessionToken() } + ); - // Get recovery codes from stored auth data - await user.fetch({ useMasterKey: true }); - const recoveryCode = user.get('authData').mfa.recovery[0]; - expect(recoveryCode).toBeDefined(); + // Get recovery codes from stored auth data + await user.fetch({ useMasterKey: true }); + const recoveryCode = user.get('authData').mfa.recovery[0]; + expect(recoveryCode).toBeDefined(); - // Send concurrent login requests with the same recovery code - const loginWithRecovery = () => - request({ - method: 'POST', - url: 'http://localhost:8378/1/login', - headers: mfaHeaders, - body: JSON.stringify({ - username: 'mfauser', - password: 'password123', - authData: { - mfa: { - token: recoveryCode, - }, + // Send concurrent login requests with the same recovery code + const loginWithRecovery = () => + request({ + method: 'POST', + url: 'http://localhost:8378/1/login', + headers: mfaHeaders, + body: JSON.stringify({ + username: 'mfauser', + password: 'password123', + authData: { + mfa: { + token: recoveryCode, }, - }), - }); + }, + }), + }); - const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery())); + const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery())); - const succeeded = results.filter(r => r.status === 'fulfilled'); - const failed = results.filter(r => r.status === 'rejected'); + const succeeded = results.filter(r => r.status === 'fulfilled'); + const failed = results.filter(r => r.status === 'rejected'); - // Exactly one request should succeed; all others should fail - expect(succeeded.length).toBe(1); - expect(failed.length).toBe(9); + // Exactly one request should succeed; all others should fail + expect(succeeded.length).toBe(1); + expect(failed.length).toBe(9); - // Verify the recovery code has been consumed - await user.fetch({ useMasterKey: true }); - const remainingRecovery = user.get('authData').mfa.recovery; - expect(remainingRecovery).not.toContain(recoveryCode); - }); + // Verify the recovery code has been consumed + await user.fetch({ useMasterKey: true }); + const remainingRecovery = user.get('authData').mfa.recovery; + expect(remainingRecovery).not.toContain(recoveryCode); }); }); diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index b99fd9f1d4..7b7fc4ed97 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -340,7 +340,10 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus patterns.push(`$${index}:raw = $${index + 1}::text`); values.push(name, fieldValue); index += 2; - } else if (typeof fieldValue === 'object') { + } else if ( + typeof fieldValue === 'object' && + !Object.keys(fieldValue).some(key => key.startsWith('$')) + ) { name = transformDotFieldToComponents(fieldName).join('->'); patterns.push(`($${index}:raw)::jsonb = $${index + 1}::jsonb`); values.push(name, JSON.stringify(fieldValue)); From 7d91f3db68df147510e650de16d9a9a723c2dc8a Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sun, 22 Mar 2026 00:45:59 +0000 Subject: [PATCH 3/3] fix: narrow optimistic locking to array fields only --- src/Routers/UsersRouter.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index fff8f3b3d0..9b63229df8 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -287,15 +287,22 @@ export class UsersRouter extends ClassesRouter { // If we have some new validated authData update directly if (validatedAuthData && Object.keys(validatedAuthData).length) { const query = { objectId: user.objectId }; - // Optimistic locking: include the original authData state in the WHERE clause + // Optimistic locking: include the original array fields in the WHERE clause // for providers whose data is being updated. This prevents concurrent requests // from both succeeding when consuming single-use tokens (e.g. MFA recovery codes). + // Only array fields need locking — element removal is vulnerable to TOCTOU; + // scalar fields are simply overwritten and don't have concurrency issues. if (user.authData) { for (const provider of Object.keys(validatedAuthData)) { const original = user.authData[provider]; - if (original && JSON.stringify(original) !== JSON.stringify(validatedAuthData[provider])) { + if (original && typeof original === 'object') { for (const [field, value] of Object.entries(original)) { - query[`authData.${provider}.${field}`] = value; + if ( + Array.isArray(value) && + JSON.stringify(value) !== JSON.stringify(validatedAuthData[provider]?.[field]) + ) { + query[`authData.${provider}.${field}`] = value; + } } } }