From 0244e3c67536613e9b3f7505672a0b706fe522d5 Mon Sep 17 00:00:00 2001 From: dcccrypto Date: Sun, 19 Jul 2026 00:09:28 +0100 Subject: [PATCH] fix(oracle-api): bound the publisher and resolver caches (GH#2416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both oracle routes used module-level Maps as TTL caches with no cap, no eviction, and no deletion of stale entries — the TTL was only a freshness check, so an expired entry was ignored but stayed strongly referenced and unreclaimable. app/api/oracle/publishers derived its cache key from request-controlled query params: const cacheKey = `${mode}:${feedId || authority || ""}`; and `authority` had no validation at all — any string, any length. So an unauthenticated caller could mint unlimited unique keys via mode=admin&authority= and grow the Map until the process died. app/api/oracle/resolve/[ca] does validate `ca` as a base58 pubkey, but any random 32 bytes is a syntactically valid pubkey, so validation alone does not bound cardinality. Each miss also fans out to Jupiter and DexScreener, making it upstream request amplification as well. Add app/lib/bounded-ttl-cache.ts and use it in both routes. It deletes expired entries on access, sweeps expired entries when the map reaches its cap, and evicts oldest-first if still at cap, so an insert can never grow it past maxEntries (500). Eviction is insertion-order rather than strict LRU — that bounds memory, which is the security property, without paying a delete+insert on every read; documented as such so nobody mistakes it for a hit-rate optimisation. Also on the publishers route: - validate `authority` as a base58 Solana pubkey (32-44 chars) and reject with 400 instead of caching it or echoing it back - serve `admin` mode uncached. getAdminPublishers is a pure local function over `authority` with no network or decoding, so caching it bought nothing and was exactly the unbounded-cardinality vector. Mirrors the eviction approach already used by createMemoryRateLimiter. Closes #2416 Co-Authored-By: Claude Opus 4.8 --- ...gh2416-oracle-publishers-cache-dos.test.ts | 84 +++++++++++++ app/__tests__/lib/bounded-ttl-cache.test.ts | 110 +++++++++++++++++ app/app/api/oracle/publishers/route.ts | 58 +++++++-- app/app/api/oracle/resolve/[ca]/route.ts | 26 ++-- app/lib/bounded-ttl-cache.ts | 112 ++++++++++++++++++ 5 files changed, 370 insertions(+), 20 deletions(-) create mode 100644 app/__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts create mode 100644 app/__tests__/lib/bounded-ttl-cache.test.ts create mode 100644 app/lib/bounded-ttl-cache.ts diff --git a/app/__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts b/app/__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts new file mode 100644 index 000000000..a9b58be53 --- /dev/null +++ b/app/__tests__/api/gh2416-oracle-publishers-cache-dos.test.ts @@ -0,0 +1,84 @@ +/** + * GH#2416 — unbounded oracle publisher cache allowed a memory-exhaustion DoS. + * + * `GET /api/oracle/publishers` built its cache key from request-controlled + * query params (`mode`, `feedId`, `authority`) and stored every result in a + * module-level `Map` with no cap, no eviction, and no deletion of stale + * entries. `mode=admin&authority=` is unauthenticated and accepted + * arbitrary strings, so a caller could mint unlimited unique keys. + */ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; +import { GET } from "@/app/api/oracle/publishers/route"; + +function req(query: string): NextRequest { + return new NextRequest(`http://localhost:3000/api/oracle/publishers?${query}`); +} + +/** A syntactically valid base58 Solana pubkey. */ +const VALID_AUTHORITY = "BXzwCWKsMpAW2MxWTWPaJu4fByYWkBFGBmLz4QxGUkwi"; + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +describe("GH#2416 oracle/publishers cache DoS", () => { + it("rejects a non-base58 authority instead of caching it", async () => { + const res = await GET(req(`mode=admin&authority=${"A".repeat(5000)}`)); + expect(res.status).toBe(400); + + const body = await res.json(); + expect(body.error).toMatch(/authority/i); + }); + + it.each([ + ["too short", "abc"], + ["non-base58 chars (0OIl)", "0OIl0OIl0OIl0OIl0OIl0OIl0OIl0OIl"], + ["oversized", "1".repeat(200)], + ["path traversal", "../../etc/passwd"], + ["empty-ish whitespace", " "], + ])("rejects malformed authority: %s", async (_label, authority) => { + const res = await GET(req(`mode=admin&authority=${encodeURIComponent(authority)}`)); + expect(res.status).toBe(400); + }); + + it("still serves a valid admin authority", async () => { + const res = await GET(req(`mode=admin&authority=${VALID_AUTHORITY}`)); + expect(res.status).toBe(200); + + const body = await res.json(); + expect(body.mode).toBe("admin"); + expect(body.publisherCount).toBe(1); + expect(body.publishers[0].key).toBe(VALID_AUTHORITY); + }); + + it("does not grow unboundedly across many unique valid authorities", async () => { + // The DoS shape, using only authorities that now pass validation. Admin + // mode is served uncached, so none of these are retained at all. + const before = process.memoryUsage().heapUsed; + + for (let i = 0; i < 300; i++) { + // Vary within the base58 alphabet to produce distinct valid-looking keys. + const authority = VALID_AUTHORITY.slice(0, -2) + ["ab", "cd", "ef", "gh"][i % 4] ; + const res = await GET(req(`mode=admin&authority=${authority}`)); + expect(res.status).toBe(200); + } + + // Not asserting an exact number — just that 300 unique unauthenticated + // requests do not retain a proportional amount of heap. + const grewMb = (process.memoryUsage().heapUsed - before) / 1024 / 1024; + expect(grewMb).toBeLessThan(50); + }); + + it("requires a mode and rejects unknown modes", async () => { + expect((await GET(req(""))).status).toBe(400); + expect((await GET(req("mode=not-a-real-mode"))).status).toBe(400); + }); + + it("still validates feedId for pyth-pinned mode", async () => { + expect((await GET(req("mode=pyth-pinned"))).status).toBe(400); + expect((await GET(req("mode=pyth-pinned&feedId=nothex"))).status).toBe(400); + expect((await GET(req(`mode=pyth-pinned&feedId=${"a".repeat(200)}`))).status).toBe(400); + }); +}); diff --git a/app/__tests__/lib/bounded-ttl-cache.test.ts b/app/__tests__/lib/bounded-ttl-cache.test.ts new file mode 100644 index 000000000..363ecbea6 --- /dev/null +++ b/app/__tests__/lib/bounded-ttl-cache.test.ts @@ -0,0 +1,110 @@ +/** + * GH#2416 — bounded TTL cache. + * + * The oracle API routes previously used plain `Map`s as TTL caches. Stale + * entries were ignored by the freshness check but never deleted, and there was + * no cap, so an unauthenticated caller who controls part of the cache key could + * mint unlimited unique keys and exhaust process memory. + */ + +import { describe, it, expect, vi, afterEach } from "vitest"; +import { createBoundedTtlCache } from "@/lib/bounded-ttl-cache"; + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("createBoundedTtlCache", () => { + it("never exceeds maxEntries no matter how many unique keys arrive", () => { + const cache = createBoundedTtlCache({ ttlMs: 60_000, maxEntries: 10 }); + + // The attack: unlimited distinct keys, all within TTL so none expire. + for (let i = 0; i < 5_000; i++) { + cache.set(`attacker-key-${i}`, i); + expect(cache.size()).toBeLessThanOrEqual(10); + } + + expect(cache.size()).toBe(10); + }); + + it("deletes an expired entry on access rather than merely ignoring it", () => { + vi.useFakeTimers(); + const cache = createBoundedTtlCache({ ttlMs: 1_000, maxEntries: 100 }); + + cache.set("k", "v"); + expect(cache.size()).toBe(1); + + vi.advanceTimersByTime(1_001); + + expect(cache.get("k")).toBeUndefined(); + // The old code left the entry in the Map here — that is the leak. + expect(cache.size()).toBe(0); + }); + + it("serves an unexpired value", () => { + vi.useFakeTimers(); + const cache = createBoundedTtlCache({ ttlMs: 5_000, maxEntries: 100 }); + + cache.set("k", "v"); + vi.advanceTimersByTime(4_999); + expect(cache.get("k")).toBe("v"); + }); + + it("treats the TTL boundary as expired", () => { + vi.useFakeTimers(); + const cache = createBoundedTtlCache({ ttlMs: 1_000, maxEntries: 100 }); + + cache.set("k", "v"); + vi.advanceTimersByTime(1_000); + expect(cache.get("k")).toBeUndefined(); + }); + + it("sweeps expired entries before evicting live ones", () => { + vi.useFakeTimers(); + const cache = createBoundedTtlCache({ ttlMs: 1_000, maxEntries: 5 }); + + for (let i = 0; i < 5; i++) cache.set(`old-${i}`, i); + expect(cache.size()).toBe(5); + + // All five lapse, so a new insert should reclaim them rather than evict. + vi.advanceTimersByTime(1_001); + cache.set("fresh", 99); + + expect(cache.size()).toBe(1); + expect(cache.get("fresh")).toBe(99); + }); + + it("evicts oldest-first when everything is still live", () => { + const cache = createBoundedTtlCache({ ttlMs: 60_000, maxEntries: 3 }); + + cache.set("a", 1); + cache.set("b", 2); + cache.set("c", 3); + cache.set("d", 4); // pushes out "a" + + expect(cache.get("a")).toBeUndefined(); + expect(cache.get("b")).toBe(2); + expect(cache.get("d")).toBe(4); + expect(cache.size()).toBe(3); + }); + + it("re-inserting a key refreshes it instead of duplicating or evicting it", () => { + const cache = createBoundedTtlCache({ ttlMs: 60_000, maxEntries: 3 }); + + cache.set("a", 1); + cache.set("b", 2); + cache.set("a", 10); // refresh — moves "a" to the end + cache.set("c", 3); + cache.set("d", 4); // evicts oldest, which is now "b" not "a" + + expect(cache.size()).toBe(3); + expect(cache.get("a")).toBe(10); + expect(cache.get("b")).toBeUndefined(); + }); + + it("rejects nonsensical construction rather than silently misbehaving", () => { + expect(() => createBoundedTtlCache({ ttlMs: 0 })).toThrow(/ttlMs/); + expect(() => createBoundedTtlCache({ ttlMs: NaN })).toThrow(/ttlMs/); + expect(() => createBoundedTtlCache({ ttlMs: 1_000, maxEntries: 0 })).toThrow(/maxEntries/); + }); +}); diff --git a/app/app/api/oracle/publishers/route.ts b/app/app/api/oracle/publishers/route.ts index 02316246e..01c3e2e0c 100644 --- a/app/app/api/oracle/publishers/route.ts +++ b/app/app/api/oracle/publishers/route.ts @@ -1,4 +1,5 @@ import { NextRequest, NextResponse } from "next/server"; +import { createBoundedTtlCache } from "@/lib/bounded-ttl-cache"; const PYTHNET_RPC = process.env.PYTHNET_RPC_URL || "https://pythnet.rpcpool.com"; @@ -26,11 +27,29 @@ const KNOWN_PYTH_PUBLISHERS: Record = { /** Max age for cached publisher data (5 minutes) */ const CACHE_TTL_MS = 5 * 60 * 1000; -interface CacheEntry { - data: PublishersResponse; - timestamp: number; +/** + * GH#2416: cap the cache. The key derives from request-controlled query + * params, so an unbounded Map lets an unauthenticated caller mint unlimited + * unique keys and exhaust process memory. `pyth-pinned` keys are additionally + * capped by the far smaller number of real Pyth feeds, so 500 is generous. + */ +const CACHE_MAX_ENTRIES = 500; + +const cache = createBoundedTtlCache({ + ttlMs: CACHE_TTL_MS, + maxEntries: CACHE_MAX_ENTRIES, +}); + +/** + * GH#2416: a Solana public key is 32 bytes, which is 32–44 base58 chars. + * Without this, `authority` was accepted as an arbitrary-length arbitrary + * string and used directly as a cache key. + */ +const BASE58_PUBKEY_RE = /^[1-9A-HJ-NP-Za-km-z]{32,44}$/; + +function isValidAuthority(authority: string): boolean { + return BASE58_PUBKEY_RE.test(authority); } -const cache = new Map(); interface PublisherInfo { key: string; @@ -65,13 +84,28 @@ export async function GET(req: NextRequest) { return NextResponse.json({ error: "Missing mode parameter" }, { status: 400 }); } - // Check cache + // GH#2416: reject a malformed authority before it can become a cache key or + // be echoed back in the response body. + if (authority !== null && !isValidAuthority(authority)) { + return NextResponse.json( + { error: "Invalid authority: expected a base58 Solana public key" }, + { status: 400 }, + ); + } + + // GH#2416: `admin` mode is a pure local function over `authority` — no + // network, no decoding. Caching it bought nothing and was precisely the + // unbounded-cardinality vector, so it is served uncached. + const isCacheable = mode !== "admin"; const cacheKey = `${mode}:${feedId || authority || ""}`; - const cached = cache.get(cacheKey); - if (cached && Date.now() - cached.timestamp < CACHE_TTL_MS) { - return NextResponse.json(cached.data, { - headers: { "Cache-Control": "public, max-age=300" }, - }); + + if (isCacheable) { + const cached = cache.get(cacheKey); + if (cached) { + return NextResponse.json(cached, { + headers: { "Cache-Control": "public, max-age=300" }, + }); + } } try { @@ -107,8 +141,8 @@ export async function GET(req: NextRequest) { return NextResponse.json({ error: `Unknown mode: ${mode}` }, { status: 400 }); } - // Update cache - cache.set(cacheKey, { data: result, timestamp: Date.now() }); + // Update cache (skipped for `admin` — see isCacheable above) + if (isCacheable) cache.set(cacheKey, result); return NextResponse.json(result, { headers: { "Cache-Control": "public, max-age=300" }, diff --git a/app/app/api/oracle/resolve/[ca]/route.ts b/app/app/api/oracle/resolve/[ca]/route.ts index d0d88058f..609e4c2f6 100644 --- a/app/app/api/oracle/resolve/[ca]/route.ts +++ b/app/app/api/oracle/resolve/[ca]/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { PublicKey } from "@solana/web3.js"; import { SUPPORTED_DEX_IDS } from "@/lib/dex-constants"; +import { createBoundedTtlCache } from "@/lib/bounded-ttl-cache"; export const dynamic = "force-dynamic"; @@ -80,13 +81,22 @@ const MINT_TO_PYTH: Record = { // Simple in-memory cache (TTL 5 minutes) // --------------------------------------------------------------------------- -interface CacheEntry { - data: OracleResolveResult; - expiresAt: number; -} -const cache = new Map(); const CACHE_TTL_MS = 5 * 60 * 1000; +/** + * GH#2416: cap the cache. `ca` is validated as a base58 pubkey below, but any + * random 32 bytes is a syntactically valid pubkey, so validation alone does + * not bound cardinality — an unauthenticated caller can still mint unlimited + * distinct keys. Each miss also fans out to Jupiter and DexScreener, so an + * unbounded map is both a memory leak and upstream request amplification. + */ +const CACHE_MAX_ENTRIES = 500; + +const cache = createBoundedTtlCache({ + ttlMs: CACHE_TTL_MS, + maxEntries: CACHE_MAX_ENTRIES, +}); + interface OracleResolveResult { feedId: string | null; symbol: string; @@ -219,8 +229,8 @@ export async function GET( // Cache hit const cached = cache.get(ca); - if (cached && Date.now() < cached.expiresAt) { - return NextResponse.json({ ...cached.data, cached: true }); + if (cached) { + return NextResponse.json({ ...cached, cached: true }); } // --- 1. Check static Pyth feed map --- @@ -273,6 +283,6 @@ export async function GET( } // Cache and return - cache.set(ca, { data: result, expiresAt: Date.now() + CACHE_TTL_MS }); + cache.set(ca, result); return NextResponse.json({ ...result, cached: false }); } diff --git a/app/lib/bounded-ttl-cache.ts b/app/lib/bounded-ttl-cache.ts new file mode 100644 index 000000000..0f0b85d42 --- /dev/null +++ b/app/lib/bounded-ttl-cache.ts @@ -0,0 +1,112 @@ +/** + * Bounded in-memory TTL cache (GH#2416). + * + * A plain `Map` used as a TTL cache leaks: when an entry goes stale it is + * ignored by the freshness check but never deleted, so both the key and the + * cached value stay strongly referenced and unreclaimable. On a route whose + * cache key derives from request input, an unauthenticated caller can mint + * unlimited unique keys and grow the map until the process dies. + * + * This bounds it three ways: + * 1. Expired entries are deleted on access, not merely ignored. + * 2. A sweep drops all expired entries once the map reaches its cap. + * 3. If the map is still at its cap after sweeping, the oldest entry is + * evicted so an insert can never grow it past `maxEntries`. + * + * Eviction is insertion-order (FIFO), not strict LRU — `set()` on an existing + * key re-inserts it at the end, but a plain `get()` does not promote. That is + * deliberate: it bounds memory, which is the security property here, without + * paying a delete+insert on every read. Do not rely on this for hit-rate + * optimisation of a hot key set. + * + * Mirrors the eviction approach already used by `createMemoryRateLimiter`. + */ + +export interface BoundedTtlCacheOptions { + /** Entry lifetime in milliseconds. */ + ttlMs: number; + /** Hard cap on entries. Default 500. */ + maxEntries?: number; +} + +export interface BoundedTtlCache { + /** Returns the value if present and unexpired; deletes it if expired. */ + get(key: string): T | undefined; + /** Stores a value, pruning and evicting as needed to respect maxEntries. */ + set(key: string, value: T): void; + /** Current entry count (may include not-yet-swept expired entries). */ + size(): number; + /** Drops everything. Intended for tests. */ + clear(): void; +} + +interface Entry { + value: T; + expiresAt: number; +} + +export function createBoundedTtlCache( + options: BoundedTtlCacheOptions, +): BoundedTtlCache { + const { ttlMs, maxEntries = 500 } = options; + + if (!Number.isFinite(ttlMs) || ttlMs <= 0) { + throw new Error(`bounded-ttl-cache: ttlMs must be a positive finite number, got ${ttlMs}`); + } + if (!Number.isFinite(maxEntries) || maxEntries <= 0) { + throw new Error( + `bounded-ttl-cache: maxEntries must be a positive finite number, got ${maxEntries}`, + ); + } + + const map = new Map>(); + + function sweepExpired(now: number): void { + for (const [k, entry] of map.entries()) { + if (now >= entry.expiresAt) map.delete(k); + } + } + + return { + get(key: string): T | undefined { + const entry = map.get(key); + if (entry === undefined) return undefined; + + if (Date.now() >= entry.expiresAt) { + // Delete rather than merely ignore — otherwise a stale entry occupies + // its slot forever and the map only ever grows. + map.delete(key); + return undefined; + } + return entry.value; + }, + + set(key: string, value: T): void { + const now = Date.now(); + + // Re-insert so a refreshed key moves to the end of the iteration order + // and is not the next eviction victim purely because it was cached early. + map.delete(key); + + if (map.size >= maxEntries) { + sweepExpired(now); + } + while (map.size >= maxEntries) { + // Map iteration order is insertion order, so the first key is oldest. + const oldest = map.keys().next(); + if (oldest.done) break; + map.delete(oldest.value); + } + + map.set(key, { value, expiresAt: now + ttlMs }); + }, + + size(): number { + return map.size; + }, + + clear(): void { + map.clear(); + }, + }; +}