From 440cc5174addd5c72398a64a7ecf1b451a12c9ea Mon Sep 17 00:00:00 2001 From: Akhila Date: Sat, 29 Aug 2026 12:39:28 +0100 Subject: [PATCH] fix(jws_verification): iterate cache keys directly in eviction loop, not Object.keys array indices --- jws_verification.ts | 7 +++++- tests/unit-tests/jws_verification.test.ts | 30 ++++++++++++++++++++++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/jws_verification.ts b/jws_verification.ts index 07b9ca8..d64ab18 100644 --- a/jws_verification.ts +++ b/jws_verification.ts @@ -262,7 +262,12 @@ export class SignedDataVerifier { if (this.enableOnlineChecks) { this.verifiedPublicKeyCache[cacheKey] = new CacheValue(leaf.publicKey, new Date().getTime() + CACHE_TIME_LIMIT) if (Object.keys(this.verifiedPublicKeyCache).length > MAXIMUM_CACHE_SIZE) { - for (let key in Object.keys(this.verifiedPublicKeyCache)) { + // `for...in` over Object.keys(...) iterates the *indices* of that + // array ("0", "1", ...), not the cache's own keys, so + // this.verifiedPublicKeyCache[key] was always undefined here and + // .cacheExpiry threw a TypeError the moment the cache grew past + // MAXIMUM_CACHE_SIZE. Iterate the cache's keys directly instead. + for (const key of Object.keys(this.verifiedPublicKeyCache)) { if (this.verifiedPublicKeyCache[key].cacheExpiry < new Date().getTime()) { delete this.verifiedPublicKeyCache[key] } diff --git a/tests/unit-tests/jws_verification.test.ts b/tests/unit-tests/jws_verification.test.ts index 38c0928..e9c8589 100644 --- a/tests/unit-tests/jws_verification.test.ts +++ b/tests/unit-tests/jws_verification.test.ts @@ -35,7 +35,18 @@ class SignedJWTVerifierTest extends SignedDataVerifier { getRootCertificates() { return this.rootCertificates - } + } + + // Seeds the (protected) verified-key cache directly with already-expired + // entries, so a subsequent verifyCertificateChain call that adds one more + // entry pushes the cache past MAXIMUM_CACHE_SIZE and exercises the + // eviction loop. + seedExpiredCacheEntries(entries: number, cacheExpiry: number) { + const dummyPublicKey = new X509Certificate(Buffer.from(LEAF_CERT_BASE64_ENCODED, 'base64')).publicKey + for (let i = 0; i < entries; i++) { + this.verifiedPublicKeyCache['test-dummy-cache-key-' + i] = { publicKey: dummyPublicKey, cacheExpiry } + } + } } describe("Chain Verification Checks", () => { @@ -175,6 +186,23 @@ describe("Chain Verification Checks", () => { jest.runOnlyPendingTimers() jest.useRealTimers() }) + + it('should evict expired cache entries once the cache grows past MAXIMUM_CACHE_SIZE, without throwing', async () => { + jest.useFakeTimers() + jest.setSystemTime(CLOCK_DATE) + const verifier = new SignedJWTVerifierTest([Buffer.from(ROOT_CA_BASE64_ENCODED, 'base64')], true, Environment.PRODUCTION, "com.example", 1234); + jest.spyOn(verifier, 'verifyCertificateChainWithoutCaching').mockImplementation((_, _2, _3, _4) => Promise.resolve(new X509Certificate(Buffer.from(LEAF_CERT_BASE64_ENCODED, 'base64')).publicKey)); + // Seed 40 already-expired cache entries directly (simulating a server + // that has verified many distinct certificate chains over time), so + // that verifying one more chain pushes the cache size past + // MAXIMUM_CACHE_SIZE (32) and triggers the eviction loop. + verifier.seedExpiredCacheEntries(40, CLOCK_DATE - 1) + await expect( + verifier.testVerifyCertificateChain(verifier.getRootCertificates(), LEAF_CERT_BASE64_ENCODED, INTERMEDIATE_CA_BASE64_ENCODED) + ).resolves.toBeDefined() + jest.runOnlyPendingTimers() + jest.useRealTimers() + }) }) describe("Decoding checks", () => {