Skip to content

Commit 5e70094

Browse files
authored
fix: MFA recovery code single-use bypass via concurrent requests ([GHSA-2299-ghjr-6vjp](GHSA-2299-ghjr-6vjp)) (#10275)
1 parent 1915f2c commit 5e70094

File tree

4 files changed

+115
-9
lines changed

4 files changed

+115
-9
lines changed

spec/vulnerabilities.spec.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4240,6 +4240,81 @@ describe('(GHSA-g4cf-xj29-wqqr) DoS via unindexed database query for unconfigure
42404240
});
42414241
});
42424242

4243+
describe('(GHSA-2299-ghjr-6vjp) MFA recovery code reuse via concurrent requests', () => {
4244+
const mfaHeaders = {
4245+
'X-Parse-Application-Id': 'test',
4246+
'X-Parse-REST-API-Key': 'rest',
4247+
'Content-Type': 'application/json',
4248+
};
4249+
4250+
beforeEach(async () => {
4251+
await reconfigureServer({
4252+
auth: {
4253+
mfa: {
4254+
enabled: true,
4255+
options: ['TOTP'],
4256+
algorithm: 'SHA1',
4257+
digits: 6,
4258+
period: 30,
4259+
},
4260+
},
4261+
});
4262+
});
4263+
4264+
it('rejects concurrent logins using the same MFA recovery code', async () => {
4265+
const OTPAuth = require('otpauth');
4266+
const user = await Parse.User.signUp('mfauser', 'password123');
4267+
const secret = new OTPAuth.Secret();
4268+
const totp = new OTPAuth.TOTP({
4269+
algorithm: 'SHA1',
4270+
digits: 6,
4271+
period: 30,
4272+
secret,
4273+
});
4274+
const token = totp.generate();
4275+
await user.save(
4276+
{ authData: { mfa: { secret: secret.base32, token } } },
4277+
{ sessionToken: user.getSessionToken() }
4278+
);
4279+
4280+
// Get recovery codes from stored auth data
4281+
await user.fetch({ useMasterKey: true });
4282+
const recoveryCode = user.get('authData').mfa.recovery[0];
4283+
expect(recoveryCode).toBeDefined();
4284+
4285+
// Send concurrent login requests with the same recovery code
4286+
const loginWithRecovery = () =>
4287+
request({
4288+
method: 'POST',
4289+
url: 'http://localhost:8378/1/login',
4290+
headers: mfaHeaders,
4291+
body: JSON.stringify({
4292+
username: 'mfauser',
4293+
password: 'password123',
4294+
authData: {
4295+
mfa: {
4296+
token: recoveryCode,
4297+
},
4298+
},
4299+
}),
4300+
});
4301+
4302+
const results = await Promise.allSettled(Array(10).fill().map(() => loginWithRecovery()));
4303+
4304+
const succeeded = results.filter(r => r.status === 'fulfilled');
4305+
const failed = results.filter(r => r.status === 'rejected');
4306+
4307+
// Exactly one request should succeed; all others should fail
4308+
expect(succeeded.length).toBe(1);
4309+
expect(failed.length).toBe(9);
4310+
4311+
// Verify the recovery code has been consumed
4312+
await user.fetch({ useMasterKey: true });
4313+
const remainingRecovery = user.get('authData').mfa.recovery;
4314+
expect(remainingRecovery).not.toContain(recoveryCode);
4315+
});
4316+
});
4317+
42434318
describe('(GHSA-p2w6-rmh7-w8q3) SQL Injection via aggregate and distinct field names in PostgreSQL adapter', () => {
42444319
const headers = {
42454320
'Content-Type': 'application/json',

src/Adapters/Storage/Mongo/MongoTransform.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,11 @@ function transformQueryKeyValue(className, key, value, schema, count = false) {
304304
return { key: 'times_used', value: value };
305305
default: {
306306
// Other auth data
307-
const authDataMatch = key.match(/^authData\.([a-zA-Z0-9_]+)\.id$/);
307+
const authDataMatch = key.match(/^authData\.([a-zA-Z0-9_]+)(\.(.+))?$/);
308308
if (authDataMatch && className === '_User') {
309309
const provider = authDataMatch[1];
310-
// Special-case auth data.
311-
return { key: `_auth_data_${provider}.id`, value };
310+
const subField = authDataMatch[3];
311+
return { key: `_auth_data_${provider}${subField ? `.${subField}` : ''}`, value };
312312
}
313313
}
314314
}

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,14 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus
340340
patterns.push(`$${index}:raw = $${index + 1}::text`);
341341
values.push(name, fieldValue);
342342
index += 2;
343+
} else if (
344+
typeof fieldValue === 'object' &&
345+
!Object.keys(fieldValue).some(key => key.startsWith('$'))
346+
) {
347+
name = transformDotFieldToComponents(fieldName).join('->');
348+
patterns.push(`($${index}:raw)::jsonb = $${index + 1}::jsonb`);
349+
values.push(name, JSON.stringify(fieldValue));
350+
index += 2;
343351
}
344352
}
345353
} else if (fieldValue === null || fieldValue === undefined) {

src/Routers/UsersRouter.js

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,35 @@ export class UsersRouter extends ClassesRouter {
286286

287287
// If we have some new validated authData update directly
288288
if (validatedAuthData && Object.keys(validatedAuthData).length) {
289-
await req.config.database.update(
290-
'_User',
291-
{ objectId: user.objectId },
292-
{ authData: validatedAuthData },
293-
{}
294-
);
289+
const query = { objectId: user.objectId };
290+
// Optimistic locking: include the original array fields in the WHERE clause
291+
// for providers whose data is being updated. This prevents concurrent requests
292+
// from both succeeding when consuming single-use tokens (e.g. MFA recovery codes).
293+
// Only array fields need locking — element removal is vulnerable to TOCTOU;
294+
// scalar fields are simply overwritten and don't have concurrency issues.
295+
if (user.authData) {
296+
for (const provider of Object.keys(validatedAuthData)) {
297+
const original = user.authData[provider];
298+
if (original && typeof original === 'object') {
299+
for (const [field, value] of Object.entries(original)) {
300+
if (
301+
Array.isArray(value) &&
302+
JSON.stringify(value) !== JSON.stringify(validatedAuthData[provider]?.[field])
303+
) {
304+
query[`authData.${provider}.${field}`] = value;
305+
}
306+
}
307+
}
308+
}
309+
}
310+
try {
311+
await req.config.database.update('_User', query, { authData: validatedAuthData }, {});
312+
} catch (error) {
313+
if (error.code === Parse.Error.OBJECT_NOT_FOUND) {
314+
throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'Invalid auth data');
315+
}
316+
throw error;
317+
}
295318
}
296319

297320
const { sessionData, createSession } = RestWrite.createSession(req.config, {

0 commit comments

Comments
 (0)