mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-31 08:56:48 +00:00
🥁 fix: Compare TOTP Codes in Constant Time (#15157)
Co-authored-by: pacocartones <pacocartones@users.noreply.github.com>
This commit is contained in:
parent
f10fcd7d19
commit
bf1e13b806
2 changed files with 69 additions and 2 deletions
|
|
@ -1,4 +1,4 @@
|
|||
const { webcrypto } = require('node:crypto');
|
||||
const { webcrypto, timingSafeEqual } = require('node:crypto');
|
||||
const { hashBackupCode, decryptV3, decryptV2 } = require('@librechat/data-schemas');
|
||||
const { updateUser } = require('~/models');
|
||||
|
||||
|
|
@ -102,6 +102,31 @@ const generateTOTP = async (secret, forTime = Date.now()) => {
|
|||
return code;
|
||||
};
|
||||
|
||||
/**
|
||||
* Constant-time comparison of a candidate 2FA code against the expected value.
|
||||
* A plain `===` comparison short-circuits at the first differing character, so
|
||||
* an attacker submitting codes to the 2FA verification endpoint could, in
|
||||
* principle, learn how many leading digits are correct from the response time.
|
||||
* Codes are of a fixed, public length, so returning early on a length mismatch
|
||||
* (or a non-string input) leaks nothing secret while keeping the match path
|
||||
* timing-independent. Mirrors the `crypto.timingSafeEqual(Buffer.from(...))`
|
||||
* pattern already used for CSRF token checks in `packages/api`.
|
||||
* @param {string} expected
|
||||
* @param {string} candidate
|
||||
* @returns {boolean}
|
||||
*/
|
||||
const constantTimeEqual = (expected, candidate) => {
|
||||
if (typeof expected !== 'string' || typeof candidate !== 'string') {
|
||||
return false;
|
||||
}
|
||||
const expectedBuffer = Buffer.from(expected, 'utf8');
|
||||
const candidateBuffer = Buffer.from(candidate, 'utf8');
|
||||
if (expectedBuffer.length !== candidateBuffer.length) {
|
||||
return false;
|
||||
}
|
||||
return timingSafeEqual(expectedBuffer, candidateBuffer);
|
||||
};
|
||||
|
||||
/**
|
||||
* Verifies a TOTP token by checking a ±1 time step window.
|
||||
* @param {string} secret
|
||||
|
|
@ -113,7 +138,7 @@ const verifyTOTP = async (secret, token) => {
|
|||
const currentTime = Date.now();
|
||||
for (let offset = -1; offset <= 1; offset++) {
|
||||
const expected = await generateTOTP(secret, currentTime + offset * timeStepMS);
|
||||
if (expected === token) {
|
||||
if (constantTimeEqual(expected, token)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
42
api/server/services/twoFactorService.spec.js
Normal file
42
api/server/services/twoFactorService.spec.js
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
const crypto = require('node:crypto');
|
||||
|
||||
jest.mock('node:crypto', () => {
|
||||
const actual = jest.requireActual('node:crypto');
|
||||
return {
|
||||
...actual,
|
||||
timingSafeEqual: jest.fn((a, b) => actual.timingSafeEqual(a, b)),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
hashBackupCode: jest.fn(),
|
||||
decryptV3: jest.fn(),
|
||||
decryptV2: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/models', () => ({ updateUser: jest.fn() }));
|
||||
|
||||
const { generateTOTP, verifyTOTP, generateTOTPSecret } = require('./twoFactorService');
|
||||
|
||||
describe('verifyTOTP', () => {
|
||||
it('accepts a valid current TOTP code', async () => {
|
||||
const secret = generateTOTPSecret();
|
||||
const code = await generateTOTP(secret);
|
||||
await expect(verifyTOTP(secret, code)).resolves.toBe(true);
|
||||
});
|
||||
|
||||
it('rejects an invalid code of the same length', async () => {
|
||||
const secret = generateTOTPSecret();
|
||||
const code = await generateTOTP(secret);
|
||||
const wrong = code === '000000' ? '111111' : '000000';
|
||||
await expect(verifyTOTP(secret, wrong)).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it('compares codes in constant time via crypto.timingSafeEqual', async () => {
|
||||
const secret = generateTOTPSecret();
|
||||
const code = await generateTOTP(secret);
|
||||
crypto.timingSafeEqual.mockClear();
|
||||
await verifyTOTP(secret, code);
|
||||
expect(crypto.timingSafeEqual).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue