diff --git a/docs/security-spec/05-integrations/squid-router.md b/docs/security-spec/05-integrations/squid-router.md index 8258d429e..52bf4294f 100644 --- a/docs/security-spec/05-integrations/squid-router.md +++ b/docs/security-spec/05-integrations/squid-router.md @@ -59,7 +59,7 @@ When the BRL on-ramp's destination is **Base + USDC**, the Nabla swap output is 2. **Bridge status uses dual-check (Squid + Axelar fallback)** — If Squid status API fails, falls back to `getStatusAxelarScan()`. Both must fail before phase errors. 3. **Balance check and bridge check MUST race via `Promise.any`** — Either balance arriving or bridge reporting success is sufficient; both must fail (`AggregateError`) to error. 4. **Arrival check MUST have a finite timeout** — `getSquidRouterPayTimeoutMs()` bounds both destination-balance and bridge-status polling at 80% of the phase processor timeout (8 minutes by default). Both checks also honor the processor's `AbortSignal`. -5. **Squid API rate-limit responses MUST be retried with backoff** — 429 responses are retried with exponential backoff before failing the phase. Other errors propagate directly. +5. **Squid API rate-limit and gateway responses MUST be retried once** — `getRoute` retries a 429 once after the advertised `retryAfter` (capped at 5 s), and a 5xx whose body is not Squid's JSON error shape (a Cloudflare / load-balancer error page) once after 1 s. Squid's own JSON errors (e.g. low liquidity) and every other error propagate directly. 6. **Axelar gas funding MUST use `addNativeGas` on the correct chain** — The funding source/chain is selected based on the route, not from request input. 7. **Permit execution MUST verify both permit and payload signatures** — `squidRouterPermitExecute` extracts v/r/s from both `permitTypedData` and `payloadTypedData`; both must be valid `SignedTypedData`. 8. **The configured executor key is the relayer caller on the source EVM network** — It funds gas only; MUST NOT hold user funds. @@ -86,7 +86,7 @@ When the BRL on-ramp's destination is **Base + USDC**, the Nabla swap output is | **Unfunded owner burns the single-use permit** | User signs the permit before funding the wallet (or drains it after signing); executing `permit()` would consume the nonce with no recoverable transfer. Backend checks `balanceOf(owner) >= value` before touching the permit and retries recoverably (~10 min window). The SDK additionally refuses registration by default; deferred-funding integrations accept responsibility for funding before transaction submission and start. | | **Executor key compromise** | Attacker can call `execute()` with their own signatures but cannot steal in-flight user funds — the key only pays gas. Blast radius: gas balance drain. | | **Squid Router API manipulation (fake "success")** | Balance check runs in parallel; even if Squid reports premature success, tokens must actually arrive. | -| **Squid rate limit (429)** | Exponential backoff retry; other errors fail fast. | +| **Squid rate limit (429) or gateway 5xx with a non-JSON body** | Single retry (`retryAfter` capped at 5 s for 429, 1 s for the gateway error); Squid's own JSON errors and all other errors fail fast. | | **Transaction not found during confirmation** | Exponential backoff retry (5s → 10s → 20s → 30s cap), up to 4 attempts. | | **No-permit fallback hash spoofing** | User reports tx hash → backend calls `waitForTransactionReceipt(hash)` and verifies the receipt `from`, receipt `to`, and transaction calldata against the expected presigned user-wallet transaction. A missing hash or mismatched transaction fails before the phase advances. | | **No-permit allowance window attack** | The `squidRouterNoPermitApprove` grants Squid an allowance from the user's wallet; if the swap hash never confirms, the allowance lingers. The user wallet, not Vortex, retains the risk. UX should remind the user to revoke unused allowances; backend cannot revoke on the user's behalf. | @@ -116,6 +116,6 @@ The removed input-currency-to-RPC fallback no longer exists. The block executor - [x] **No-permit fallback receipt validation**: `waitForUserHash` verifies receipt `from`, receipt `to`, and transaction `input` against the expected user address and presigned EVM transaction payload before advancing. - [x] **Skip-Squid trivial path**: the block catalog selects the direct flow for exact same-chain corridors; direct quote simulation preserves zero network fee and transaction preparation omits Squid phases. **PASS** — no security checks bypassed. - [x] **Destination-token raw output metadata**: `evmToEvm.outputAmountRaw` preserves Squid's `route.estimate.toAmount` in destination raw units, including routed Alfredpay onramps. **PASS** — prevents Base/Polygon 6-decimal source → BSC USDT-style 18-decimal destination under-scaling. -- [x] **Squid 429 rate-limit retry**: exponential backoff. **PASS — verify backoff cap.** +- [x] **Squid 429 and gateway 5xx retry**: a single retry, after the advertised `retryAfter` capped at 5 s (`MAX_RETRY_AFTER_MS`) for a 429 and after 1 s for a 5xx with a non-JSON body; Squid's own JSON errors fail fast. **PASS** - [x] **Arrival timeout**: `waitUntilTrue` accepts a timeout argument. **PASS** — verify all callers pass a finite value. - [EXISTING FINDING F-054]: `backupSquidRouterApprove`/`backupSquidRouterSwap`/`backupApprove` presigned txs have no registered phase handler. Either dead code or missing implementation. diff --git a/packages/shared/src/services/squidrouter/route.test.ts b/packages/shared/src/services/squidrouter/route.test.ts index d7fc1edaa..20976f16a 100644 --- a/packages/shared/src/services/squidrouter/route.test.ts +++ b/packages/shared/src/services/squidrouter/route.test.ts @@ -19,6 +19,35 @@ afterEach(() => { globalThis.fetch = realFetch; }); +const validRouteBody = { + route: { + estimate: { + aggregateSlippage: 1, + toAmount: "1000000", + toAmountMin: "990000", + toAmountUSD: "1", + toToken: { decimals: 6 } + }, + quoteId: "quote-1", + transactionRequest: { + data: "0x", + gasLimit: "350000", + target: "0x5000000000000000000000000000000000000005", + value: "1000000" + } + } +}; + +function fetchSequence(responses: Response[]): { calls: number } { + const state = { calls: 0 }; + globalThis.fetch = (async () => { + const response = responses[state.calls] ?? responses[responses.length - 1]; + state.calls += 1; + return response; + }) as unknown as typeof fetch; + return state; +} + describe("getRoute response validation", () => { test("rejects malformed executable route terms before returning them", async () => { globalThis.fetch = (async () => @@ -44,3 +73,40 @@ describe("getRoute response validation", () => { await expect(getRoute(params)).rejects.toThrow(); }); }); + +describe("getRoute upstream error handling", () => { + test("retries once when the gateway in front of Squid answers a non-JSON 5xx", async () => { + const state = fetchSequence([ + new Response("502 Bad Gateway", { status: 502 }), + Response.json(validRouteBody) + ]); + + const result = await getRoute(params); + + expect(result.data.route.quoteId).toBe("quote-1"); + expect(state.calls).toBe(2); + }); + + test("retries a 5xx whose body cannot be read instead of dropping the status", async () => { + const truncated = new ReadableStream({ + start(controller) { + controller.error(new TypeError("terminated")); + } + }); + const state = fetchSequence([new Response(truncated, { status: 502 }), Response.json(validRouteBody)]); + + const result = await getRoute(params); + + expect(result.data.route.quoteId).toBe("quote-1"); + expect(state.calls).toBe(2); + }); + + test("does not retry Squid's own JSON errors and surfaces their message", async () => { + const state = fetchSequence([ + Response.json({ message: "Low liquidity, please reduce swap amount and try again", statusCode: 500 }, { status: 500 }) + ]); + + await expect(getRoute(params)).rejects.toThrow("Failed to fetch route: Low liquidity"); + expect(state.calls).toBe(1); + }); +}); diff --git a/packages/shared/src/services/squidrouter/route.ts b/packages/shared/src/services/squidrouter/route.ts index b01878d26..8edd131e8 100644 --- a/packages/shared/src/services/squidrouter/route.ts +++ b/packages/shared/src/services/squidrouter/route.ts @@ -116,6 +116,7 @@ const routeQueues = new Map(); // Cap any retryAfter value Squidrouter returns to avoid pathologically long waits if the API misbehaves. const MAX_RETRY_AFTER_MS = 5000; +const TRANSIENT_RETRY_DELAY_MS = 1000; class HttpError extends Error { status: number; @@ -131,7 +132,15 @@ class HttpError extends Error { async function squidFetch(url: string, options: RequestInit): Promise<{ data: T; headers: Headers }> { const response = await fetch(url, options); if (!response.ok) { - const errorData = await response.json().catch(() => ({})); + // Squid's own errors are JSON; keep a non-JSON body (Cloudflare / load-balancer error page) as text. + // A body that cannot be read (truncated stream) must still surface the status, so treat it as empty. + const text = await response.text().catch(() => ""); + let errorData: unknown = text; + try { + errorData = JSON.parse(text); + } catch { + // not JSON: the raw text is the most useful thing to log and marks the error as a gateway error + } throw new HttpError(response.status, errorData); } const data = (await response.json()) as T; @@ -194,15 +203,22 @@ async function getRouteInternalWithRetry(params: RouteParams): Promise= 500 && typeof error.data === "string"; +} + function extractRateLimitRetryAfterMs(error: unknown): number | undefined { if (!(error instanceof HttpError)) return undefined; @@ -241,7 +257,7 @@ async function getRouteInternal(params: RouteParams): Promise