Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: M-06 Incorrect Parameters Passed to permitWitnessTransferFrom #745

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Nov 11, 2024

The PERMIT2_ORDER_TYPE variable stores the witness data type string, which is supposed to be passed as the witnessTypeString parameter to the permitWitnessTransferFrom function of the Permit2 contract. As such, this variable is supposed to define the typed data that the witness parameter passed to that function was hashed from. However, it instead specifies that the witness parameter has been hashed from the CrossChainOrder type, whereas in reality, it was hashed from the GaslessCrossChainOrder type.

Moreover, the witness parameter specified is incorrect as the orderDataType member of the GaslessCrossChainOrder struct is not taken into account when calculating it. The same is true for the exclusiveRelayer and depositNonce members of the AcrossOrderData struct, which are not included in the calculation of its hash. Furthermore, the CROSS_CHAIN_ORDER_TYPE variable used to create the witness contains an incorrect encoding of the GaslessCrossChainOrder struct as the originChainId member is specified to be of type uint32 instead of uint64.

Consider correcting the errors described above in order to maintain compliance with EIP-712 and Permit2.

The issues with originChainId being a uint32 was fixed in this PR, so this proposal just modifies the PERMIT2_ORDER_TYPE and adds the necessary fields to the order hash.

@bmzig bmzig marked this pull request as ready for review November 14, 2024 20:53
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