mirror of https://github.com/garrytan/gstack.git
fix(token-registry): UTF-8 byte-length short-circuit before timingSafeEqual
isRootToken's length check compared JS string lengths, but the buffers passed to crypto.timingSafeEqual are UTF-8 byte representations. A multibyte token whose JS string length matches rootToken (e.g. "é".repeat(20), 20 chars / 40 bytes, vs a 20-byte ASCII root) would slip past the length check and crash inside timingSafeEqual on the mismatched-buffer-size invariant, turning a 401/403 auth-rejection path into an unhandled exception. Switch the short-circuit to Buffer.byteLength(token, 'utf8') and build the comparison buffers with explicit 'utf8' encoding. Regression tests cover: multibyte input with matching string length but differing byte length (the actual bug, asserts not.toThrow plus false), differing-length tokens (short-circuit branch), same-length differ-by- last-byte (timingSafeEqual branch), and empty string when root is set. Caught by Codex during pre-landing review of #1171.
This commit is contained in:
parent
5d4fe7df07
commit
fa754092e3
|
|
@ -155,7 +155,20 @@ export function getRootToken(): string {
|
|||
}
|
||||
|
||||
export function isRootToken(token: string): boolean {
|
||||
return token === rootToken;
|
||||
// Constant-time compare so a tunnel-reachable caller who can provoke an
|
||||
// isRootToken() call (e.g., via the 403 "root over tunnel" rejection path)
|
||||
// can't measure byte-by-byte string-compare timing to recover the token.
|
||||
// Compare UTF-8 byte lengths (not JS string length) before timingSafeEqual,
|
||||
// which throws on length-mismatched buffers. A multibyte input whose JS
|
||||
// string length matches rootToken but whose UTF-8 byte length differs must
|
||||
// return false on the auth path, not error out.
|
||||
if (!rootToken) return false;
|
||||
const tokenBytes = Buffer.byteLength(token, 'utf8');
|
||||
const rootBytes = Buffer.byteLength(rootToken, 'utf8');
|
||||
if (tokenBytes !== rootBytes) return false;
|
||||
const a = Buffer.from(token, 'utf8');
|
||||
const b = Buffer.from(rootToken, 'utf8');
|
||||
return crypto.timingSafeEqual(a, b);
|
||||
}
|
||||
|
||||
function generateToken(prefix: string): string {
|
||||
|
|
|
|||
|
|
@ -28,6 +28,39 @@ describe('token-registry', () => {
|
|||
expect(info!.scopes).toEqual(['read', 'write', 'admin', 'meta', 'control']);
|
||||
expect(info!.rateLimit).toBe(0);
|
||||
});
|
||||
|
||||
// Regression: the previous fix did a JS string-length short-circuit before
|
||||
// crypto.timingSafeEqual, but the buffers passed in are UTF-8. A multibyte
|
||||
// input with matching string length but mismatched byte length would slip
|
||||
// past the check and crash inside timingSafeEqual. Auth path must return
|
||||
// false, not error.
|
||||
it('returns false for a multibyte token whose string length matches but UTF-8 byte length differs', () => {
|
||||
// 'root-token-for-tests' is 20 ASCII chars (20 bytes).
|
||||
// 'é'.repeat(20) is 20 chars but 40 UTF-8 bytes.
|
||||
const multibyte = 'é'.repeat(20);
|
||||
expect(multibyte.length).toBe('root-token-for-tests'.length);
|
||||
expect(Buffer.byteLength(multibyte, 'utf8')).not.toBe(
|
||||
Buffer.byteLength('root-token-for-tests', 'utf8'),
|
||||
);
|
||||
expect(() => isRootToken(multibyte)).not.toThrow();
|
||||
expect(isRootToken(multibyte)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for a token that differs only in length (same prefix)', () => {
|
||||
expect(isRootToken('root-token-for-tests-extra')).toBe(false);
|
||||
expect(isRootToken('root-token-for-test')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for a same-length token that differs only in the last byte', () => {
|
||||
const expected = 'root-token-for-tests';
|
||||
const wrong = expected.slice(0, -1) + (expected.endsWith('x') ? 'y' : 'x');
|
||||
expect(wrong.length).toBe(expected.length);
|
||||
expect(isRootToken(wrong)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for the empty string even when root is set', () => {
|
||||
expect(isRootToken('')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createToken', () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue