Skip to content

fix(transfer): reject a malformed MPP challenge request parameter - #147

Open
rayasa07 wants to merge 1 commit into
tempoxyz:mainfrom
rayasa07:fix/reject-malformed-mpp-challenge-request
Open

rayasa07 wants to merge 1 commit into
tempoxyz:mainfrom
rayasa07:fix/reject-malformed-mpp-challenge-request

Conversation

@rayasa07

Copy link
Copy Markdown

parseMppChallenge hands the request parameter of a WWW-Authenticate: Payment … challenge straight to JSON.parse:

export function decodeBase64UrlJson(value: string) {
  return getRecord(JSON.parse(Buffer.from(value, "base64url").toString("utf8")));
}

The value comes from whatever server answered with 402, so it is not trusted input. When it is not base64url-encoded JSON, the SyntaxError escapes transferCredits uncaught and printCompatErrorAndExit falls back to its defaults — E_RUNTIME, exit code 1 — with a raw parser message:

$ tempo wallet transfer --credits --mpp-challenge 'Payment realm="example", method="tempo", intent="charge", id="abc", request="bm90LWpzb24"'
Unexpected token 'o', "not-json" is not valid JSON

Every other malformed-challenge condition in that function raises usageError, so a caller that branches on exit code 2 or E_USAGE to tell "the server sent me a bad challenge" from "the CLI broke" gets the wrong answer here. Buffer.from(value, "base64url") does not reject invalid characters either, so request="!!!!" decodes to empty and produces Unexpected end of JSON input the same way.

A request that parses but is not an object has the opposite problem. getRecord turns null, [] or 5 into {}, which is truthy, so the existing if (!request) guard cannot catch it — only a completely absent parameter reaches it. The challenge is accepted with an empty request and fails several checks later with a message that points at the wrong field:

Invalid configuration: MPP challenge currency  does not match token 0x20c0…8b50 for chain 4217

Fix

Decode the parameter through a local helper that returns null for unparseable JSON and for JSON that is not an object, and raise usageError on it. The absent-parameter check moves ahead of the decode so it keeps its existing "Missing request parameter." message, and the new case reports "Malformed request parameter."

decodeBase64UrlJson itself is unchanged. Its other caller, storedSessionChallenge in src/commands/sessions.ts, already wraps the decode in try/catch and reads from the local channel store rather than from the network.

Tests

Adds four cases to test/transfer.test.ts, alongside the existing MPP challenge validation tests: request that is not JSON, request that is not base64url, request that decodes to null / [] / 5 / "amount", and a challenge carrying no request at all.

The first three fail on main — the first two as an uncaught SyntaxError, the third on the misleading currency message. The fourth passes either way and is there to pin the untouched "Missing request parameter." path.

Validation

vitest run test/transfer.test.ts — 14 passed, up from 10. pnpm typecheck, pnpm test:types, pnpm check:lint and pnpm changelog:validate are clean.

The full suite is not a usable signal on my machine: 7 of its 19 files fail on an unmodified main — spawn sqlite3 ENOENT, since sqlite3 is not available here and I cannot install it, plus 10s timeouts — and the same 7 fail identically on this branch (37 failed / 208 passed both ways, 249 tests on the branch against 245 on main). One branch run timed out in three further files, mcp-services, request-io and wallet-readiness; all three pass when run on their own on this branch, and none of them touch src/commands/transfer.ts. pnpm check:format also fails on main, on .agents/friction-log/20260831180632-immutable-releases-were/friction.md, which this branch does not touch.

The request parameter of an MPP challenge went straight to JSON.parse, so a
challenge whose request was not base64url-encoded JSON escaped parseMppChallenge
as an uncaught SyntaxError and exited 1 under E_RUNTIME, while every other
invalid-challenge check raises E_USAGE and exits 2. A request that decoded to
valid JSON that was not an object was also accepted, degrading to an empty
record and failing later on a missing currency.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant