Symptom
frontend/src/settings.ts:603 defines:
function generateExternalID(): string {
if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') {
return crypto.randomUUID();
}
return `cudly-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`;
}The fallback path uses Math.random(), which is not cryptographically secure — V8's implementation effectively yields fewer than 30 bits of entropy in practice from a single sample, and the output stream is recoverable from a handful of observations. The External ID is a security-relevant shared secret that protects against the confused-deputy problem on cross-account sts:AssumeRole. Generating it from a weak PRNG defeats the protection in the cases where the fallback is hit.
Why a fallback exists at all
crypto.randomUUID is missing in some test environments and a few obscure webviews. The intent of the fallback is "never hand the user an empty field". That's correct — but the fallback should still be cryptographically strong.
Fix
Use crypto.getRandomValues(new Uint8Array(16)) and hex-encode (or format as a UUID v4). getRandomValues has a strict superset of the browser support randomUUID has, so the fallback effectively becomes:
function generateExternalID(): string {
if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') {
return crypto.randomUUID();
}
if (typeof crypto !== 'undefined' && typeof crypto.getRandomValues === 'function') {
const bytes = new Uint8Array(16);
crypto.getRandomValues(bytes);
return Array.from(bytes, b => b.toString(16).padStart(2, '0')).join('');
}
// Last-resort, only hits if the runtime has no Web Crypto at all
// (jsdom without polyfill etc.). Not security-secret-quality but
// never empty, and the field still gets validated server-side.
return `cudly-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`;
}The deepest fallback can stay as a defence against truly empty fields, but the primary fallback should be CSPRNG-grade.
Severity
Medium. Real users hitting the Math.random() path is rare given browser support of randomUUID, but: (a) the field is security-relevant; (b) the fix is trivial; (c) backend validation alone can't catch a weak-but-non-empty value.
Test coverage gap
No test asserts that the generated External ID has high entropy. Easy regression test: assert non-collision across N=10000 generations and that values match /^[0-9a-f]{32}$|^[0-9a-f-]{36}$/i.
Related
Symptom
frontend/src/settings.ts:603defines:The fallback path uses
Math.random(), which is not cryptographically secure — V8's implementation effectively yields fewer than 30 bits of entropy in practice from a single sample, and the output stream is recoverable from a handful of observations. The External ID is a security-relevant shared secret that protects against the confused-deputy problem on cross-accountsts:AssumeRole. Generating it from a weak PRNG defeats the protection in the cases where the fallback is hit.Why a fallback exists at all
crypto.randomUUIDis missing in some test environments and a few obscure webviews. The intent of the fallback is "never hand the user an empty field". That's correct — but the fallback should still be cryptographically strong.Fix
Use
crypto.getRandomValues(new Uint8Array(16))and hex-encode (or format as a UUID v4).getRandomValueshas a strict superset of the browser supportrandomUUIDhas, so the fallback effectively becomes:The deepest fallback can stay as a defence against truly empty fields, but the primary fallback should be CSPRNG-grade.
Severity
Medium. Real users hitting the
Math.random()path is rare given browser support ofrandomUUID, but: (a) the field is security-relevant; (b) the fix is trivial; (c) backend validation alone can't catch a weak-but-non-empty value.Test coverage gap
No test asserts that the generated External ID has high entropy. Easy regression test: assert non-collision across N=10000 generations and that values match
/^[0-9a-f]{32}$|^[0-9a-f-]{36}$/i.Related