Skip to content

Commit e7efbeb

Browse files
authored
fix: MFA single-use token bypass via concurrent authData login requests ([GHSA-w73w-g5xw-rwhf](GHSA-w73w-g5xw-rwhf)) (#10326)
1 parent 2be73d9 commit e7efbeb

File tree

2 files changed

+117
-1
lines changed

2 files changed

+117
-1
lines changed

spec/vulnerabilities.spec.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4315,6 +4315,91 @@ describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests'
43154315
});
43164316
});
43174317

4318+
describe('(GHSA-w73w-g5xw-rwhf) MFA recovery code reuse via concurrent authData-only login', () => {
4319+
const mfaHeaders = {
4320+
'X-Parse-Application-Id': 'test',
4321+
'X-Parse-REST-API-Key': 'rest',
4322+
'Content-Type': 'application/json',
4323+
};
4324+
4325+
let fakeProvider;
4326+
4327+
beforeEach(async () => {
4328+
fakeProvider = {
4329+
validateAppId: () => Promise.resolve(),
4330+
validateAuthData: () => Promise.resolve(),
4331+
};
4332+
await reconfigureServer({
4333+
auth: {
4334+
fakeProvider,
4335+
mfa: {
4336+
enabled: true,
4337+
options: ['TOTP'],
4338+
algorithm: 'SHA1',
4339+
digits: 6,
4340+
period: 30,
4341+
},
4342+
},
4343+
});
4344+
});
4345+
4346+
it('rejects concurrent authData-only logins using the same MFA recovery code', async () => {
4347+
const OTPAuth = require('otpauth');
4348+
4349+
// Create user via authData login with fake provider
4350+
const user = await Parse.User.logInWith('fakeProvider', {
4351+
authData: { id: 'user1', token: 'fakeToken' },
4352+
});
4353+
4354+
// Enable MFA for this user
4355+
const secret = new OTPAuth.Secret();
4356+
const totp = new OTPAuth.TOTP({
4357+
algorithm: 'SHA1',
4358+
digits: 6,
4359+
period: 30,
4360+
secret,
4361+
});
4362+
const token = totp.generate();
4363+
await user.save(
4364+
{ authData: { mfa: { secret: secret.base32, token } } },
4365+
{ sessionToken: user.getSessionToken() }
4366+
);
4367+
4368+
// Get recovery codes from stored auth data
4369+
await user.fetch({ useMasterKey: true });
4370+
const recoveryCode = user.get('authData').mfa.recovery[0];
4371+
expect(recoveryCode).toBeDefined();
4372+
4373+
// Send concurrent authData-only login requests with the same recovery code
4374+
const loginWithRecovery = () =>
4375+
request({
4376+
method: 'POST',
4377+
url: 'http://localhost:8378/1/users',
4378+
headers: mfaHeaders,
4379+
body: JSON.stringify({
4380+
authData: {
4381+
fakeProvider: { id: 'user1', token: 'fakeToken' },
4382+
mfa: { token: recoveryCode },
4383+
},
4384+
}),
4385+
});
4386+
4387+
const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery()));
4388+
4389+
const succeeded = results.filter(r => r.status === 'fulfilled');
4390+
const failed = results.filter(r => r.status === 'rejected');
4391+
4392+
// Exactly one request should succeed; all others should fail
4393+
expect(succeeded.length).toBe(1);
4394+
expect(failed.length).toBe(9);
4395+
4396+
// Verify the recovery code has been consumed
4397+
await user.fetch({ useMasterKey: true });
4398+
const remainingRecovery = user.get('authData').mfa.recovery;
4399+
expect(remainingRecovery).not.toContain(recoveryCode);
4400+
});
4401+
});
4402+
43184403
describe('(GHSA-p2w6-rmh7-w8q3) SQL Injection via aggregate and distinct field names in PostgreSQL adapter', () => {
43194404
const headers = {
43204405
'Content-Type': 'application/json',

src/RestWrite.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,15 @@ RestWrite.prototype.handleAuthData = async function (authData) {
663663
// We are supposed to have a response only on LOGIN with authData, so we skip those
664664
// If we're not logging in, but just updating the current user, we can safely skip that part
665665
if (this.response) {
666+
// Capture original authData before mutating userResult via the response reference
667+
const originalAuthData = userResult?.authData
668+
? Object.fromEntries(
669+
Object.entries(userResult.authData).map(([k, v]) =>
670+
[k, v && typeof v === 'object' ? { ...v } : v]
671+
)
672+
)
673+
: undefined;
674+
666675
// Assign the new authData in the response
667676
Object.keys(mutatedAuthData).forEach(provider => {
668677
this.response.response.authData[provider] = mutatedAuthData[provider];
@@ -673,14 +682,36 @@ RestWrite.prototype.handleAuthData = async function (authData) {
673682
// uses the `doNotSave` option. Just update the authData part
674683
// Then we're good for the user, early exit of sorts
675684
if (Object.keys(this.data.authData).length) {
685+
const query = { objectId: this.data.objectId };
686+
// Optimistic locking: include the original array fields in the WHERE clause
687+
// for providers whose data is being updated. This prevents concurrent requests
688+
// from both succeeding when consuming single-use tokens (e.g. MFA recovery codes).
689+
if (originalAuthData) {
690+
for (const provider of Object.keys(this.data.authData)) {
691+
const original = originalAuthData[provider];
692+
if (original && typeof original === 'object') {
693+
for (const [field, value] of Object.entries(original)) {
694+
if (
695+
Array.isArray(value) &&
696+
JSON.stringify(value) !== JSON.stringify(this.data.authData[provider]?.[field])
697+
) {
698+
query[`authData.${provider}.${field}`] = value;
699+
}
700+
}
701+
}
702+
}
703+
}
676704
try {
677705
await this.config.database.update(
678706
this.className,
679-
{ objectId: this.data.objectId },
707+
query,
680708
{ authData: this.data.authData },
681709
{}
682710
);
683711
} catch (error) {
712+
if (error.code === Parse.Error.OBJECT_NOT_FOUND) {
713+
throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'Invalid auth data');
714+
}
684715
this._throwIfAuthDataDuplicate(error);
685716
throw error;
686717
}

0 commit comments

Comments
 (0)