From da2e4846d3564e3c29d46f35d2ce5dceb691c06d Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 26 Mar 2026 21:18:20 +0000 Subject: [PATCH] fix: GHSA-w73w-g5xw-rwhf v9 --- spec/vulnerabilities.spec.js | 85 ++++++++++++++++++++++++++++++++++++ src/RestWrite.js | 33 +++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 9763ea77f2..70d6eeca22 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -4315,6 +4315,91 @@ describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests' }); }); +describe('(GHSA-w73w-g5xw-rwhf) MFA recovery code reuse via concurrent authData-only login', () => { + const mfaHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'Content-Type': 'application/json', + }; + + let fakeProvider; + + beforeEach(async () => { + fakeProvider = { + validateAppId: () => Promise.resolve(), + validateAuthData: () => Promise.resolve(), + }; + await reconfigureServer({ + auth: { + fakeProvider, + mfa: { + enabled: true, + options: ['TOTP'], + algorithm: 'SHA1', + digits: 6, + period: 30, + }, + }, + }); + }); + + it('rejects concurrent authData-only logins using the same MFA recovery code', async () => { + const OTPAuth = require('otpauth'); + + // Create user via authData login with fake provider + const user = await Parse.User.logInWith('fakeProvider', { + authData: { id: 'user1', token: 'fakeToken' }, + }); + + // Enable MFA for this user + 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 authData-only login requests with the same recovery code + const loginWithRecovery = () => + request({ + method: 'POST', + url: 'http://localhost:8378/1/users', + headers: mfaHeaders, + body: JSON.stringify({ + authData: { + fakeProvider: { id: 'user1', token: 'fakeToken' }, + 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', () => { const headers = { 'Content-Type': 'application/json', diff --git a/src/RestWrite.js b/src/RestWrite.js index 00d4eb858a..5260359300 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -663,6 +663,15 @@ RestWrite.prototype.handleAuthData = async function (authData) { // We are supposed to have a response only on LOGIN with authData, so we skip those // If we're not logging in, but just updating the current user, we can safely skip that part if (this.response) { + // Capture original authData before mutating userResult via the response reference + const originalAuthData = userResult?.authData + ? Object.fromEntries( + Object.entries(userResult.authData).map(([k, v]) => + [k, v && typeof v === 'object' ? { ...v } : v] + ) + ) + : undefined; + // Assign the new authData in the response Object.keys(mutatedAuthData).forEach(provider => { this.response.response.authData[provider] = mutatedAuthData[provider]; @@ -673,14 +682,36 @@ RestWrite.prototype.handleAuthData = async function (authData) { // uses the `doNotSave` option. Just update the authData part // Then we're good for the user, early exit of sorts if (Object.keys(this.data.authData).length) { + const query = { objectId: this.data.objectId }; + // 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). + if (originalAuthData) { + for (const provider of Object.keys(this.data.authData)) { + const original = originalAuthData[provider]; + if (original && typeof original === 'object') { + for (const [field, value] of Object.entries(original)) { + if ( + Array.isArray(value) && + JSON.stringify(value) !== JSON.stringify(this.data.authData[provider]?.[field]) + ) { + query[`authData.${provider}.${field}`] = value; + } + } + } + } + } try { await this.config.database.update( this.className, - { objectId: this.data.objectId }, + query, { authData: this.data.authData }, {} ); } catch (error) { + if (error.code === Parse.Error.OBJECT_NOT_FOUND) { + throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'Invalid auth data'); + } this._throwIfAuthDataDuplicate(error); throw error; }