Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/security-spec/05-integrations/squid-router.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
ebma marked this conversation as resolved.
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.
Expand All @@ -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. |
Expand Down Expand Up @@ -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.
66 changes: 66 additions & 0 deletions packages/shared/src/services/squidrouter/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () =>
Expand All @@ -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("<html>502 Bad Gateway</html>", { 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);
});
});
24 changes: 20 additions & 4 deletions packages/shared/src/services/squidrouter/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ const routeQueues = new Map<string, PQueue>();

// 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;
Expand All @@ -131,7 +132,15 @@ class HttpError extends Error {
async function squidFetch<T>(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;
Expand Down Expand Up @@ -194,15 +203,22 @@ async function getRouteInternalWithRetry(params: RouteParams): Promise<Squidrout
try {
return await getRouteInternal(params);
} catch (error) {
const retryAfterMs = extractRateLimitRetryAfterMs(error);
const retryAfterMs =
extractRateLimitRetryAfterMs(error) ?? (isTransientUpstreamError(error) ? TRANSIENT_RETRY_DELAY_MS : undefined);
Comment thread
ebma marked this conversation as resolved.
if (retryAfterMs === undefined) throw error;

logger.current.warn(`Squidrouter rate limit hit. Retrying once after ${retryAfterMs}ms.`);
logger.current.warn(`Squidrouter route request failed transiently. Retrying once after ${retryAfterMs}ms.`);
await sleep(retryAfterMs);
return getRouteInternal(params);
}
}

// A 5xx whose body is not Squid's JSON error shape comes from the gateway in front of Squid
// (observed ~1-2 times per hour in production); a single retry is cheap for this read-only query.
function isTransientUpstreamError(error: unknown): boolean {
return error instanceof HttpError && error.status >= 500 && typeof error.data === "string";
}

function extractRateLimitRetryAfterMs(error: unknown): number | undefined {
if (!(error instanceof HttpError)) return undefined;

Expand Down Expand Up @@ -241,7 +257,7 @@ async function getRouteInternal(params: RouteParams): Promise<SquidrouterRouteRe
});
} catch (error) {
if (error instanceof HttpError) {
logger.current.error(`Error fetching route from Squidrouter API: ${JSON.stringify(error.data)}`);
logger.current.error(`Error fetching route from Squidrouter API: HTTP ${error.status} ${JSON.stringify(error.data)}`);
const message =
typeof error.data === "object" && error.data !== null && "message" in error.data
? String((error.data as { message: unknown }).message)
Expand Down
Loading