diff --git a/src/__tests__/eip3009-auth.test.ts b/src/__tests__/eip3009-auth.test.ts index 628dc2c..a6c5330 100644 --- a/src/__tests__/eip3009-auth.test.ts +++ b/src/__tests__/eip3009-auth.test.ts @@ -278,4 +278,45 @@ describe("eip3009AuthenticatedFetch", () => { expect(res.status).toBe(402) expect(fetchMock).toHaveBeenCalledTimes(1) }) + + it("does not sign a value-bearing 402 challenge", async () => { + let callCount = 0 + const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => { + callCount++ + // A hostile endpoint demands a real USDC transfer instead of a + // zero-value proof of wallet control. + return new Response( + JSON.stringify({ + x402Version: 1, + accepts: [ + { + scheme: "exact", + network: "base", + maxAmountRequired: "500000000", + payTo: OPERATOR, + asset: "0x833589fcd6edb6e08f4c7c32d4f71b54bda02913", + extra: { name: "USD Coin", version: "2" }, + }, + ], + }), + { status: 402 }, + ) + }) + vi.stubGlobal("fetch", fetchMock) + + const { eip3009AuthenticatedFetch } = await import( + "../lib/client/eip3009-auth.js" + ) + + const res = await eip3009AuthenticatedFetch( + "https://tool.example.com/api", + { account, method: "POST", body: "{}" }, + ) + + // The value-bearing requirement must not be signed or replayed: the 402 is + // returned as-is and there is no second (paying) request. + expect(res.status).toBe(402) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(callCount).toBe(1) + }) }) diff --git a/src/__tests__/pay.test.ts b/src/__tests__/pay.test.ts index f15c43d..a5d9328 100644 --- a/src/__tests__/pay.test.ts +++ b/src/__tests__/pay.test.ts @@ -88,6 +88,61 @@ describe("pay command", () => { logSpy.mockRestore() }) + it("refuses to sign when the server asset is not the expected USDC", async () => { + const calls: { headers: Record }[] = [] + const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + const headers = Object.fromEntries( + Object.entries(init?.headers ?? {}), + ) as Record + calls.push({ headers }) + return new Response( + JSON.stringify({ + x402Version: 1, + error: "Payment required", + accepts: [ + { + ...PAYMENT_REQUIREMENTS, + // Hostile endpoint asks the caller to sign a transfer of an + // attacker-controlled token instead of USDC. + asset: "0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + }, + ], + }), + { status: 402 }, + ) + }) + vi.stubGlobal("fetch", fetchMock) + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { + throw new Error("process.exit") + }) as never) + process.env.PRIVATE_KEY = PRIVATE_KEY + process.env.RPC_URL = "http://localhost:8545" + + const { payCommand } = await import("../cli/commands/pay.js") + + await expect( + payCommand.parseAsync([ + "node", + "pay", + "https://tool.example.com/api", + "--body", + "{}", + ]), + ).rejects.toThrow() + + // The probe happened, but no payment authorization was signed or replayed. + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(calls[0].headers["X-Payment"]).toBeUndefined() + expect(exitSpy).toHaveBeenCalledWith(1) + + logSpy.mockRestore() + errSpy.mockRestore() + exitSpy.mockRestore() + }) + it("prints response without payment when endpoint does not return 402", async () => { vi.stubGlobal( "fetch", diff --git a/src/cli/commands/pay.ts b/src/cli/commands/pay.ts index 11904e2..2717fba 100644 --- a/src/cli/commands/pay.ts +++ b/src/cli/commands/pay.ts @@ -4,7 +4,10 @@ import { type Address, getAddress } from "viem" import { createExternalSignerAccount } from "../../lib/client/external-signer.js" import { paidAuthenticatedFetch } from "../../lib/client/paid-authenticated-fetch.js" import type { PaymentRequirements } from "../../lib/client/x402-payment.js" -import { signX402Payment } from "../../lib/client/x402-payment.js" +import { + signX402Payment, + validatePaymentRequirements, +} from "../../lib/client/x402-payment.js" import { ToolManifestSchema } from "../../lib/manifest/schema.js" import { createWalletForProvider, @@ -16,12 +19,17 @@ import { import { loadManifest } from "./load-manifest.js" import { readInput } from "./read-input.js" +// 1 USDC (6 decimals). The payment amount is dictated by the server's 402 +// response; this caps what `pay` will sign without an explicit override. +const DEFAULT_MAX_AMOUNT = "1000000" + interface PayOptions { body?: string auth?: string manifest?: string chain?: string walletProvider?: string + maxAmount?: string } export const payCommand = new Command("pay") @@ -47,6 +55,10 @@ export const payCommand = new Command("pay") "--wallet-provider ", `Wallet provider: ${WALLET_PROVIDERS.join(", ")}`, ) + .option( + "--max-amount ", + `Maximum payment amount to sign, in the token's smallest unit (default: ${DEFAULT_MAX_AMOUNT})`, + ) .action(async (url: string, options: PayOptions) => { let useAuth = options.auth === "eip3009" || options.auth === "siwe" @@ -127,7 +139,7 @@ export const payCommand = new Command("pay") if (useAuth) { await runPaidAuthenticated(url, inputBody, adapter) } else { - await runPaymentOnly(url, inputBody, adapter) + await runPaymentOnly(url, inputBody, adapter, options.maxAmount) } }) @@ -202,6 +214,7 @@ async function runPaymentOnly( url: string, inputBody: string, adapter: WalletAdapter, + maxAmount?: string, ): Promise { console.log(pc.cyan("Probing endpoint for payment requirements...")) @@ -255,6 +268,22 @@ async function runPaymentOnly( console.log(` Pay To: ${requirements.payTo}`) console.log(` Asset: ${requirements.asset}`) + // The amount, recipient, and asset above come from the (untrusted) server's + // 402 response. Validate them before signing a transfer authorization, the + // same way `paidFetch` does, so a hostile endpoint cannot have us sign a + // transfer for an arbitrary asset/amount to an arbitrary address. + try { + validatePaymentRequirements(requirements, { + maxAmount: maxAmount ?? DEFAULT_MAX_AMOUNT, + }) + } catch (err) { + console.error(pc.red(err instanceof Error ? err.message : String(err))) + console.error( + pc.dim("Pass --max-amount to raise the cap if this payment is expected."), + ) + process.exit(1) + } + console.log(pc.cyan("\nSigning EIP-3009 transferWithAuthorization...")) const xPayment = await signX402Payment({ diff --git a/src/lib/client/eip3009-auth.ts b/src/lib/client/eip3009-auth.ts index bfc2815..c0677ae 100644 --- a/src/lib/client/eip3009-auth.ts +++ b/src/lib/client/eip3009-auth.ts @@ -97,7 +97,12 @@ async function extractPaymentRequirements( if ( reqs?.payTo && reqs.payTo !== "0x0000000000000000000000000000000000000000" && - reqs.scheme === "exact" + reqs.scheme === "exact" && + // This helper signs a zero-value authorization (proof of wallet control), + // so refuse to sign anything that would move funds. A value-bearing + // requirement belongs to the paid flow (`paidFetch`), which validates the + // recipient, asset, and amount before signing. + (reqs.maxAmountRequired === undefined || reqs.maxAmountRequired === "0") ) { return reqs } diff --git a/src/lib/client/x402-payment.ts b/src/lib/client/x402-payment.ts index 033363a..d67b19f 100644 --- a/src/lib/client/x402-payment.ts +++ b/src/lib/client/x402-payment.ts @@ -233,7 +233,7 @@ export async function paidFetch( return paidRes } -function validatePaymentRequirements( +export function validatePaymentRequirements( reqs: PaymentRequirements, opts: { maxAmount?: string