Skip to content

Commit 531b9ab

Browse files
authored
fix: Login timing side-channel reveals user existence ([GHSA-mmpq-5hcv-hf2v](GHSA-mmpq-5hcv-hf2v)) (#10398)
1 parent f7f3542 commit 531b9ab

File tree

3 files changed

+41
-1
lines changed

3 files changed

+41
-1
lines changed

spec/ParseUser.spec.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,29 @@ describe('Parse.User testing', () => {
8282
}
8383
});
8484

85+
it('normalizes login response time for non-existent and existing users', async () => {
86+
const passwordCrypto = require('../lib/password');
87+
const compareSpy = spyOn(passwordCrypto, 'compare').and.callThrough();
88+
await Parse.User.signUp('existinguser', 'password123');
89+
compareSpy.calls.reset();
90+
91+
// Login with non-existent user — should use dummy hash
92+
await expectAsync(
93+
Parse.User.logIn('nonexistentuser', 'wrongpassword')
94+
).toBeRejected();
95+
expect(compareSpy).toHaveBeenCalledTimes(1);
96+
expect(compareSpy).toHaveBeenCalledWith('wrongpassword', passwordCrypto.dummyHash);
97+
compareSpy.calls.reset();
98+
99+
// Login with existing user but wrong password — should use real hash
100+
await expectAsync(
101+
Parse.User.logIn('existinguser', 'wrongpassword')
102+
).toBeRejected();
103+
expect(compareSpy).toHaveBeenCalledTimes(1);
104+
expect(compareSpy.calls.mostRecent().args[0]).toBe('wrongpassword');
105+
expect(compareSpy.calls.mostRecent().args[1]).not.toBe(passwordCrypto.dummyHash);
106+
});
107+
85108
it('logs username taken with configured log level', async () => {
86109
await reconfigureServer({ logLevels: { signupUsernameTaken: 'warn' } });
87110
const logger = require('../lib/logger').default;

src/Routers/UsersRouter.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,13 @@ export class UsersRouter extends ClassesRouter {
108108
.find('_User', query, {}, Auth.maintenance(req.config))
109109
.then(results => {
110110
if (!results.length) {
111-
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Invalid username/password.');
111+
// Perform a dummy bcrypt compare to normalize response timing,
112+
// preventing user enumeration via timing side-channel
113+
return passwordCrypto
114+
.compare(password, passwordCrypto.dummyHash)
115+
.then(() => {
116+
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Invalid username/password.');
117+
});
112118
}
113119

114120
if (results.length > 1) {
@@ -121,6 +127,11 @@ export class UsersRouter extends ClassesRouter {
121127
user = results[0];
122128
}
123129

130+
if (typeof user.password !== 'string' || user.password.length === 0) {
131+
// Passwordless account (e.g. OAuth-only): run dummy compare for
132+
// timing normalization, discard result, always reject
133+
return passwordCrypto.compare(password, passwordCrypto.dummyHash).then(() => false);
134+
}
124135
return passwordCrypto.compare(password, user.password);
125136
})
126137
.then(correct => {

src/password.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@ function compare(password, hashedPassword) {
2727
return bcrypt.compare(password, hashedPassword);
2828
}
2929

30+
// Pre-computed bcrypt hash (cost factor 10) used for timing normalization.
31+
// The actual value is irrelevant; it ensures bcrypt.compare() runs with
32+
// realistic cost even when no real password hash is available.
33+
const dummyHash = '$2b$10$Wd1gvrMYPnQv5pHBbXCwCehxXmJSEzRqNON0ev98L6JJP5296S35i';
34+
3035
module.exports = {
3136
hash: hash,
3237
compare: compare,
38+
dummyHash: dummyHash,
3339
};

0 commit comments

Comments
 (0)