Skip to content

Commit 83e90ed

Browse files
authored
fix: Endpoints /login and /verifyPassword disclose MFA secrets and protected fields when _User get is denied ([GHSA-75v4-m273-5j49](GHSA-75v4-m273-5j49)) (#10492)
1 parent 2b9d93d commit 83e90ed

2 files changed

Lines changed: 163 additions & 4 deletions

File tree

spec/vulnerabilities.spec.js

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6151,4 +6151,142 @@ describe('Vulnerabilities', () => {
61516151
expect(req.info.clientSDK).toBeUndefined();
61526152
});
61536153
});
6154+
6155+
describe('(GHSA-75v4-m273-5j49) _User CLP refetch fallback leaks raw MFA secrets and protected fields', () => {
6156+
const headers = {
6157+
'X-Parse-Application-Id': 'test',
6158+
'X-Parse-REST-API-Key': 'rest',
6159+
'Content-Type': 'application/json',
6160+
};
6161+
6162+
const denyGetCLP = {
6163+
get: {},
6164+
find: {},
6165+
create: { '*': true },
6166+
update: { '*': true },
6167+
delete: {},
6168+
};
6169+
6170+
const updateUserCLP = classLevelPermissions =>
6171+
request({
6172+
method: 'PUT',
6173+
url: Parse.serverURL + '/schemas/_User',
6174+
headers: {
6175+
'X-Parse-Application-Id': 'test',
6176+
'X-Parse-Master-Key': 'test',
6177+
'Content-Type': 'application/json',
6178+
},
6179+
body: JSON.stringify({ classLevelPermissions }),
6180+
});
6181+
6182+
async function setupMfaUser() {
6183+
const OTPAuth = require('otpauth');
6184+
const user = await Parse.User.signUp('victim', 'password');
6185+
const sessionToken = user.getSessionToken();
6186+
user.set('phone', '555-1234');
6187+
await user.save(null, { sessionToken });
6188+
const secret = new OTPAuth.Secret();
6189+
const totp = new OTPAuth.TOTP({ algorithm: 'SHA1', digits: 6, period: 30, secret });
6190+
await user.save(
6191+
{ authData: { mfa: { secret: secret.base32, token: totp.generate() } } },
6192+
{ sessionToken }
6193+
);
6194+
return { user, totp, secret };
6195+
}
6196+
6197+
beforeEach(async () => {
6198+
await reconfigureServer({
6199+
auth: {
6200+
mfa: { enabled: true, options: ['TOTP'], algorithm: 'SHA1', digits: 6, period: 30 },
6201+
},
6202+
protectedFields: { _User: { '*': ['phone'] } },
6203+
protectedFieldsOwnerExempt: false,
6204+
});
6205+
});
6206+
6207+
it('does not leak raw MFA secrets or protected fields from /verifyPassword when _User get CLP denies the re-fetch', async () => {
6208+
await setupMfaUser();
6209+
await updateUserCLP(denyGetCLP);
6210+
6211+
const response = await request({
6212+
method: 'POST',
6213+
url: Parse.serverURL + '/verifyPassword',
6214+
headers,
6215+
body: JSON.stringify({ username: 'victim', password: 'password' }),
6216+
});
6217+
6218+
expect(response.status).toBe(200);
6219+
expect(response.data.objectId).toBeDefined();
6220+
// Access control denied the re-fetch, so no stored fields may be disclosed
6221+
expect(response.data.authData).toBeUndefined();
6222+
expect(response.data.phone).toBeUndefined();
6223+
});
6224+
6225+
it('does not leak raw MFA secrets or protected fields from /login when _User get CLP denies the re-fetch', async () => {
6226+
const { totp } = await setupMfaUser();
6227+
await updateUserCLP(denyGetCLP);
6228+
6229+
const response = await request({
6230+
method: 'POST',
6231+
url: Parse.serverURL + '/login',
6232+
headers,
6233+
body: JSON.stringify({
6234+
username: 'victim',
6235+
password: 'password',
6236+
authData: { mfa: { token: totp.generate() } },
6237+
}),
6238+
});
6239+
6240+
expect(response.status).toBe(200);
6241+
// Login still succeeds and issues a session for the authenticated user
6242+
expect(response.data.objectId).toBeDefined();
6243+
expect(response.data.sessionToken).toBeDefined();
6244+
// But discloses no stored fields the caller may not read
6245+
expect(response.data.authData).toBeUndefined();
6246+
expect(response.data.phone).toBeUndefined();
6247+
});
6248+
6249+
it('sanitizes MFA secrets and protected fields on /verifyPassword when get CLP permits the re-fetch', async () => {
6250+
await setupMfaUser();
6251+
6252+
const response = await request({
6253+
method: 'POST',
6254+
url: Parse.serverURL + '/verifyPassword',
6255+
headers,
6256+
body: JSON.stringify({ username: 'victim', password: 'password' }),
6257+
});
6258+
6259+
expect(response.status).toBe(200);
6260+
expect(response.data.objectId).toBeDefined();
6261+
// afterFind replaces raw MFA material with a status flag
6262+
expect(response.data.authData.mfa.status).toBe('enabled');
6263+
expect(response.data.authData.mfa.secret).toBeUndefined();
6264+
expect(response.data.authData.mfa.recovery).toBeUndefined();
6265+
// protectedFieldsOwnerExempt:false strips protected fields even for the owner
6266+
expect(response.data.phone).toBeUndefined();
6267+
});
6268+
6269+
it('returns the full user to a master-key /verifyPassword even when get CLP is denied', async () => {
6270+
await setupMfaUser();
6271+
await updateUserCLP(denyGetCLP);
6272+
6273+
const response = await request({
6274+
method: 'POST',
6275+
url: Parse.serverURL + '/verifyPassword',
6276+
headers: {
6277+
'X-Parse-Application-Id': 'test',
6278+
'X-Parse-Master-Key': 'test',
6279+
'Content-Type': 'application/json',
6280+
},
6281+
body: JSON.stringify({ username: 'victim', password: 'password' }),
6282+
});
6283+
6284+
expect(response.status).toBe(200);
6285+
expect(response.data.objectId).toBeDefined();
6286+
// Master bypasses CLP and protectedFields by design, so it still receives
6287+
// the full record (auth hierarchy preserved); the minimal denied-path
6288+
// response only applies to non-master callers.
6289+
expect(response.data.phone).toBe('555-1234');
6290+
});
6291+
});
61546292
});

src/Routers/UsersRouter.js

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,10 +370,21 @@ export class UsersRouter extends ClassesRouter {
370370
);
371371
filteredUser = filteredUserResponse.results?.[0];
372372
} catch {
373-
// re-fetch may fail for legacy users without ACL; fall through
373+
// The re-fetch enforces `_User` `get` CLP and may be denied by access
374+
// control (e.g. CLP `get: {}` or an ACL that excludes the caller).
375+
// Handled below; never fall back to the raw row.
374376
}
375377
if (!filteredUser) {
376-
filteredUser = user;
378+
// Master/maintenance callers bypass CLP, protectedFields, and authData
379+
// afterFind, so for them an empty re-fetch is a genuine not-found edge, not
380+
// an access-control denial; they are entitled to the full row. For every
381+
// other caller, an empty/denied re-fetch means access control withheld the
382+
// record, so disclose only the identity — never the raw row, which would
383+
// leak fields hidden by `protectedFields` and raw `authData` (e.g. MFA
384+
// secrets and recovery codes) that the sanitizing re-fetch would remove.
385+
// The session token is still attached below so login succeeds.
386+
filteredUser =
387+
req.auth.isMaster || req.auth.isMaintenance ? user : { objectId: user.objectId };
377388
}
378389
UsersRouter.removeHiddenProperties(filteredUser);
379390
filteredUser.sessionToken = user.sessionToken;
@@ -472,10 +483,20 @@ export class UsersRouter extends ClassesRouter {
472483
);
473484
filteredUser = filteredUserResponse.results?.[0];
474485
} catch {
475-
// re-fetch may fail for legacy users without ACL; fall through
486+
// The re-fetch enforces `_User` `get` CLP and may be denied by access
487+
// control (e.g. CLP `get: {}` or an ACL that excludes the caller).
488+
// Handled below; never fall back to the raw row.
476489
}
477490
if (!filteredUser) {
478-
filteredUser = user;
491+
// See handleLogIn: master/maintenance callers bypass CLP,
492+
// protectedFields, and authData afterFind, so an empty re-fetch is a
493+
// genuine not-found edge for them and they are entitled to the full
494+
// row. For all other callers, an empty/denied re-fetch means access
495+
// control withheld the record, so disclose only the identity rather
496+
// than the raw row, which would leak protectedFields and raw authData
497+
// (e.g. MFA secrets and recovery codes).
498+
filteredUser =
499+
req.auth.isMaster || req.auth.isMaintenance ? user : { objectId: user.objectId };
479500
}
480501
UsersRouter.removeHiddenProperties(filteredUser);
481502
return { response: filteredUser };

0 commit comments

Comments
 (0)