Skip to content

Commit 2d2b2fa

Browse files
committed
1 parent 1915f2c commit 2d2b2fa

File tree

4 files changed

+105
-9
lines changed

4 files changed

+105
-9
lines changed

spec/vulnerabilities.spec.js

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

42434318
describe('(GHSA-p2w6-rmh7-w8q3) SQL Injection via aggregate and distinct field names in PostgreSQL adapter', () => {

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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,11 @@ 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 (typeof fieldValue === 'object') {
344+
name = transformDotFieldToComponents(fieldName).join('->');
345+
patterns.push(`($${index}:raw)::jsonb = $${index + 1}::jsonb`);
346+
values.push(name, JSON.stringify(fieldValue));
347+
index += 2;
343348
}
344349
}
345350
} else if (fieldValue === null || fieldValue === undefined) {

src/Routers/UsersRouter.js

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,28 @@ 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 authData state 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+
if (user.authData) {
294+
for (const provider of Object.keys(validatedAuthData)) {
295+
const original = user.authData[provider];
296+
if (original && JSON.stringify(original) !== JSON.stringify(validatedAuthData[provider])) {
297+
for (const [field, value] of Object.entries(original)) {
298+
query[`authData.${provider}.${field}`] = value;
299+
}
300+
}
301+
}
302+
}
303+
try {
304+
await req.config.database.update('_User', query, { authData: validatedAuthData }, {});
305+
} catch (error) {
306+
if (error.code === Parse.Error.OBJECT_NOT_FOUND) {
307+
throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'Invalid auth data');
308+
}
309+
throw error;
310+
}
295311
}
296312

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

0 commit comments

Comments
 (0)