Conversation
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.
_recoverPaymentSignaturehad the session key signhashSessionAuthorization(wallet, policyRoot, payload.digest), wherepayload.digestis the raw Permit2 digest. That digest carries no owner and the authorization ignoredpayload.parentWallets, so the tuple the signer saw for a direct payment from a wallet was identical to the tuple it saw when the same wallet was reached as a sapient signer of a parent wallet. The same signature bytes validated for both owners, and since Permit2 nonces are per owner both transfers could settle off one authorization. The bytes are public in calldata after the first settlement, so anyone could do the second one.The payment path now signs
Payload.hashFor(payload, wallet), the same wallet-scoped hash the approval path already used. That commits to the Permit2 digest, thenoChainIdflag,parentWalletsand the wallet domain, so a payment authorized for one wallet no longer validates through a parent that delegates to it, and vice versa. Every other leaf type was already protected because the wallet opHash commits toparentWallets; only this path skipped it.Ran
forge buildandforge test --match-path "test/extensions/x402/*"on foundry v1.5.1: 49 passed, 0 failed. That is the 47 existing tests, whose signing helpers now sign the wallet-scoped hash, plus two new ones covering the nesting cases. I confirmed both new tests fail against the old code. I did not run the rest of the suite because it needs an external RPC server.