Skip to content

Commit 55d8f5b

Browse files
committed
fix(x402): validate server payment requirements before signing
The amount, recipient, and asset in an x402 402 response are server-controlled. `paidFetch` validates them before signing (validatePaymentRequirements: payTo is not a burn address, asset defaults to the network USDC, optional maxAmount/allowedRecipients), but two other client paths did not: - `pay` (runPaymentOnly) passed the server's requirements straight to signX402Payment with no checks, so a hostile endpoint could have the caller sign a transfer of an arbitrary asset/amount to an arbitrary address. It now runs validatePaymentRequirements with a default 1 USDC cap, overridable via --max-amount (mirroring `smoke`). - eip3009AuthenticatedFetch is documented as signing a zero-value proof of wallet control, but it forwarded the server's maxAmountRequired to signX402Payment, so a value-bearing requirement was signed. It now signs only when the requirement is zero-value; a value-bearing 402 is returned as-is. validatePaymentRequirements is exported so `pay` can reuse it. Adds regression tests for both paths.
1 parent 94f2f2f commit 55d8f5b

5 files changed

Lines changed: 134 additions & 4 deletions

File tree

src/__tests__/eip3009-auth.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,4 +278,45 @@ describe("eip3009AuthenticatedFetch", () => {
278278
expect(res.status).toBe(402)
279279
expect(fetchMock).toHaveBeenCalledTimes(1)
280280
})
281+
282+
it("does not sign a value-bearing 402 challenge", async () => {
283+
let callCount = 0
284+
const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => {
285+
callCount++
286+
// A hostile endpoint demands a real USDC transfer instead of a
287+
// zero-value proof of wallet control.
288+
return new Response(
289+
JSON.stringify({
290+
x402Version: 1,
291+
accepts: [
292+
{
293+
scheme: "exact",
294+
network: "base",
295+
maxAmountRequired: "500000000",
296+
payTo: OPERATOR,
297+
asset: "0x833589fcd6edb6e08f4c7c32d4f71b54bda02913",
298+
extra: { name: "USD Coin", version: "2" },
299+
},
300+
],
301+
}),
302+
{ status: 402 },
303+
)
304+
})
305+
vi.stubGlobal("fetch", fetchMock)
306+
307+
const { eip3009AuthenticatedFetch } = await import(
308+
"../lib/client/eip3009-auth.js"
309+
)
310+
311+
const res = await eip3009AuthenticatedFetch(
312+
"https://tool.example.com/api",
313+
{ account, method: "POST", body: "{}" },
314+
)
315+
316+
// The value-bearing requirement must not be signed or replayed: the 402 is
317+
// returned as-is and there is no second (paying) request.
318+
expect(res.status).toBe(402)
319+
expect(fetchMock).toHaveBeenCalledTimes(1)
320+
expect(callCount).toBe(1)
321+
})
281322
})

src/__tests__/pay.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,61 @@ describe("pay command", () => {
8888
logSpy.mockRestore()
8989
})
9090

91+
it("refuses to sign when the server asset is not the expected USDC", async () => {
92+
const calls: { headers: Record<string, string> }[] = []
93+
const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => {
94+
const headers = Object.fromEntries(
95+
Object.entries(init?.headers ?? {}),
96+
) as Record<string, string>
97+
calls.push({ headers })
98+
return new Response(
99+
JSON.stringify({
100+
x402Version: 1,
101+
error: "Payment required",
102+
accepts: [
103+
{
104+
...PAYMENT_REQUIREMENTS,
105+
// Hostile endpoint asks the caller to sign a transfer of an
106+
// attacker-controlled token instead of USDC.
107+
asset: "0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
108+
},
109+
],
110+
}),
111+
{ status: 402 },
112+
)
113+
})
114+
vi.stubGlobal("fetch", fetchMock)
115+
116+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {})
117+
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {})
118+
const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {
119+
throw new Error("process.exit")
120+
}) as never)
121+
process.env.PRIVATE_KEY = PRIVATE_KEY
122+
process.env.RPC_URL = "http://localhost:8545"
123+
124+
const { payCommand } = await import("../cli/commands/pay.js")
125+
126+
await expect(
127+
payCommand.parseAsync([
128+
"node",
129+
"pay",
130+
"https://tool.example.com/api",
131+
"--body",
132+
"{}",
133+
]),
134+
).rejects.toThrow()
135+
136+
// The probe happened, but no payment authorization was signed or replayed.
137+
expect(fetchMock).toHaveBeenCalledTimes(1)
138+
expect(calls[0].headers["X-Payment"]).toBeUndefined()
139+
expect(exitSpy).toHaveBeenCalledWith(1)
140+
141+
logSpy.mockRestore()
142+
errSpy.mockRestore()
143+
exitSpy.mockRestore()
144+
})
145+
91146
it("prints response without payment when endpoint does not return 402", async () => {
92147
vi.stubGlobal(
93148
"fetch",

src/cli/commands/pay.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { type Address, getAddress } from "viem"
44
import { createExternalSignerAccount } from "../../lib/client/external-signer.js"
55
import { paidAuthenticatedFetch } from "../../lib/client/paid-authenticated-fetch.js"
66
import type { PaymentRequirements } from "../../lib/client/x402-payment.js"
7-
import { signX402Payment } from "../../lib/client/x402-payment.js"
7+
import {
8+
signX402Payment,
9+
validatePaymentRequirements,
10+
} from "../../lib/client/x402-payment.js"
811
import { ToolManifestSchema } from "../../lib/manifest/schema.js"
912
import {
1013
createWalletForProvider,
@@ -16,12 +19,17 @@ import {
1619
import { loadManifest } from "./load-manifest.js"
1720
import { readInput } from "./read-input.js"
1821

22+
// 1 USDC (6 decimals). The payment amount is dictated by the server's 402
23+
// response; this caps what `pay` will sign without an explicit override.
24+
const DEFAULT_MAX_AMOUNT = "1000000"
25+
1926
interface PayOptions {
2027
body?: string
2128
auth?: string
2229
manifest?: string
2330
chain?: string
2431
walletProvider?: string
32+
maxAmount?: string
2533
}
2634

2735
export const payCommand = new Command("pay")
@@ -47,6 +55,10 @@ export const payCommand = new Command("pay")
4755
"--wallet-provider <provider>",
4856
`Wallet provider: ${WALLET_PROVIDERS.join(", ")}`,
4957
)
58+
.option(
59+
"--max-amount <amount>",
60+
`Maximum payment amount to sign, in the token's smallest unit (default: ${DEFAULT_MAX_AMOUNT})`,
61+
)
5062
.action(async (url: string, options: PayOptions) => {
5163
let useAuth = options.auth === "eip3009" || options.auth === "siwe"
5264

@@ -127,7 +139,7 @@ export const payCommand = new Command("pay")
127139
if (useAuth) {
128140
await runPaidAuthenticated(url, inputBody, adapter)
129141
} else {
130-
await runPaymentOnly(url, inputBody, adapter)
142+
await runPaymentOnly(url, inputBody, adapter, options.maxAmount)
131143
}
132144
})
133145

@@ -202,6 +214,7 @@ async function runPaymentOnly(
202214
url: string,
203215
inputBody: string,
204216
adapter: WalletAdapter,
217+
maxAmount?: string,
205218
): Promise<void> {
206219
console.log(pc.cyan("Probing endpoint for payment requirements..."))
207220

@@ -255,6 +268,22 @@ async function runPaymentOnly(
255268
console.log(` Pay To: ${requirements.payTo}`)
256269
console.log(` Asset: ${requirements.asset}`)
257270

271+
// The amount, recipient, and asset above come from the (untrusted) server's
272+
// 402 response. Validate them before signing a transfer authorization, the
273+
// same way `paidFetch` does, so a hostile endpoint cannot have us sign a
274+
// transfer for an arbitrary asset/amount to an arbitrary address.
275+
try {
276+
validatePaymentRequirements(requirements, {
277+
maxAmount: maxAmount ?? DEFAULT_MAX_AMOUNT,
278+
})
279+
} catch (err) {
280+
console.error(pc.red(err instanceof Error ? err.message : String(err)))
281+
console.error(
282+
pc.dim("Pass --max-amount to raise the cap if this payment is expected."),
283+
)
284+
process.exit(1)
285+
}
286+
258287
console.log(pc.cyan("\nSigning EIP-3009 transferWithAuthorization..."))
259288

260289
const xPayment = await signX402Payment({

src/lib/client/eip3009-auth.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,12 @@ async function extractPaymentRequirements(
9797
if (
9898
reqs?.payTo &&
9999
reqs.payTo !== "0x0000000000000000000000000000000000000000" &&
100-
reqs.scheme === "exact"
100+
reqs.scheme === "exact" &&
101+
// This helper signs a zero-value authorization (proof of wallet control),
102+
// so refuse to sign anything that would move funds. A value-bearing
103+
// requirement belongs to the paid flow (`paidFetch`), which validates the
104+
// recipient, asset, and amount before signing.
105+
(reqs.maxAmountRequired === undefined || reqs.maxAmountRequired === "0")
101106
) {
102107
return reqs
103108
}

src/lib/client/x402-payment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ export async function paidFetch(
233233
return paidRes
234234
}
235235

236-
function validatePaymentRequirements(
236+
export function validatePaymentRequirements(
237237
reqs: PaymentRequirements,
238238
opts: {
239239
maxAmount?: string

0 commit comments

Comments
 (0)