refactor + audit hardening: boilerplate dedup + 6 security fixes - #47
Merged
Conversation
… hardenings
- swap exact-in: assert BigInt(quote.action.fromAmount) equals parseUnits(args.amount, decimals);
without this, a hostile/MITM'd LiFi quote could inflate fromAmount (toAmount scaled
proportionally to dodge the toUsd/fromUsd>10 gate) and drive the approval/swap bytes
to pull a different amount than the MCP preview displays
- prepareNativeSend / prepareTokenSend: reject mixed-case recipient with a wrong EIP-55
checksum via isAddress(x, { strict: true }); all-lowercase still accepted, catches
single-character case typos before the bytes reach Ledger
- preview_send: return the existing pin verbatim unless refresh:true is passed; the
pre-sign hash the user matched in chat cannot silently drift before Ledger displays it
- TRON USB: serialize getTronLedgerAddress / signTronTxOnLedger through a module-local
promise chain so concurrent HID opens queue instead of colliding on the exclusive
transport
- setup: label printed pairing URI as sensitive / one-time
- Dockerfile: drop to USER node in the runtime stage
- test/audit/01-*: regression test for the VP-01 fromAmount assert + happy-path guard
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Summary
src/modules/shared/{token-meta,approval}.ts.prepareSwapnow assertsBigInt(quote.action.fromAmount) === parseUnits(args.amount, decimals)right after the decimals cross-check. Without this, a hostile/MITM'd LiFi response could inflatefromAmount(withtoAmountscaled proportionally to dodge thetoUsd/fromUsd > 10gate) so the approval + swap bytes pull more than the MCP preview displays.prepareNativeSend/prepareTokenSendnow reject mixed-case recipients with a wrong EIP-55 checksum viaisAddress(x, { strict: true }); all-lowercase still accepted. Catches single-character case typos before bytes reach Ledger.preview_sendpin guard: re-calls return the existing pin verbatim unlessrefresh: trueis passed. ThepreSignHashthe user matched in chat can no longer silently drift before Ledger displays it.getTronLedgerAddress/signTronTxOnLedgerrun under a module-local promise-chain lock so concurrent HID opens queue instead of colliding on the exclusive transport.(sensitive — don't share; one-time pairing secret).USER node.test/audit/01-swap-exact-in-approval-divergence.test.tscovers the Medium fix (refusal on drift + happy-path regression).Test plan
npx vitest run— 444/444 tests passnpx tsc --noEmit— cleanprepare_swap→preview_send→send_transactionon testnet or small mainnet amount, confirm no regression in the normal flowpreview_sendtwice on the same handle, confirm second call returns the samepreSignHashwithoutrefresh: trueprepare_native_sendwith a mixed-case-wrong-checksum recipient, confirm refusal🤖 Generated with Claude Code