Harden local trust boundary and fix signing-path bugs - #4
Merged
Conversation
The proxy triggers wallet signing and exposes pending-transaction data, but bound to all interfaces with wildcard CORS and no Origin/Host validation, so any website the developer visited (or any machine on their LAN) could inject transactions, read in-flight requests, and forge results back to the dev tool. Server: - Add a local-only guard: reject non-loopback Host (DNS-rebinding) and foreign Origin (CSRF); non-browser tools send no Origin and still pass through. - Bind the listener to 127.0.0.1 and validate --port. - Remove the /api/pending enumeration endpoint that leaked all live request IDs. - Cap batch size (50) and concurrent pending requests (100) to bound DoS. - Guard /api/complete JSON parsing and require a boolean success field. - Clear the request timeout on resolve and unref it; return JSON 404 for unknown /api/* paths instead of serving the SPA HTML. Web: - personal_sign/eth_sign: sign raw bytes for hex messages instead of the UTF-8 of the "0x..." string, so the signature matches what the dapp expects. - Always notify the server on failure so the calling script gets an immediate rejection instead of blocking until the 5-minute timeout. - Treat post-send completion errors as bookkeeping, not transaction failures. - Add a synchronous re-entrancy guard so a double-click can't double-broadcast. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The proxy triggers wallet signing and exposes pending-transaction data, but it bound to all network interfaces with wildcard CORS and no Origin/Host validation. That means any website the developer visited — or any machine on the same LAN — could:
eth_sendTransactionthat auto-opens a wallet-approval tab (atext/plain"simple" request skips the CORS preflight, so the side effect fires regardless),to/value/dataof in-flight signing requests via the/api/pendingenumeration endpoint, and/api/complete.This PR closes that boundary and fixes several correctness bugs in the signing path found during the same review.
What changed
Server — trust boundary
Hostisn't loopback (localhost/127.0.0.1/[::1]:<port>, blocking DNS rebinding) or whoseOriginis present and foreign (blocking CSRF). Non-browser tools send noOriginand pass through, so Foundry/Hardhat/curl keep working.127.0.0.1(index.ts) so the server is unreachable off-machine; also validates--port./api/pendingenumeration (and deadgetAllPendingIds/getPendingCount) — the unguessable UUID is now the only handle to a request./api/completehardening: guarded JSON parse (clean 400, not 500) and a required booleansuccess.unref; unknown/api/*returns JSON 404 instead of the SPA page.Web — signing path
personal_sign/eth_signsign raw bytes for hex messages ({ raw }) instead of the UTF-8 of the"0x…"string, so the signature verifies against what the dapp expects and the wallet popup matches the review screen.useRef) so a double-click can't broadcast twice.Verification
bun run build(server + web) andtsc --noEmitboth clean.eth_chainIdworks; cross-origineth_sendTransaction→ 403; foreignHost→ 403;/api/pending→ 404 JSON (no ID leak); 51-item batch → 400; malformed/empty/api/complete→ 400; valid routes + SPA still 200.Notes for reviewers
Originas allowed on purpose — that's how non-browser dev tools reach the RPC. Browsers always sendOriginon cross-origin POSTs, so the CSRF path is covered.bun install --frozen-lockfile, the./typesexport pointing at unshipped source) and remaining web display-fidelity fixes (chain label vstx.chainId,formatUnitsdecimals,generate-chains.tscurrency coupling, an error boundary, surfacing nonce/fees).🤖 Generated with Claude Code