Permit2Transfers Manual Review Findings

PTS-01M: Inexistent Validation of Native Amount

Type	Severity	Location
Input Sanitization		Permit2Transfers.sol:L32

Description:

The Permit2Transfers::takeFromCaller function does not validate that sufficient native funds were sent alongside the call if the _token specified is the NATIVE_TOKEN of the Token contract.

Impact:

It is possible for functions that rely on Permit2Transfers::takeFromCaller invocations to incorrectly assume they possess a msg.value equal to the _amount input when it is not the case.

As an example, the SwapPermit2Adapter::sellOrderSwap function can be invoked with an arbitrary _params.amountIn value and will yield it in its return without the msg.value necessarily equating it.

Example:

Copy
22function takeFromCaller(
23  IPermit2 _permit2,
24  address _token,
25  uint256 _amount,
26  uint256 _nonce,
27  uint256 _deadline,
28  bytes calldata _signature
29)
30  internal
31{
32  if (address(_token) != Token.NATIVE_TOKEN) {
33    _permit2.permitTransferFrom(
34      // The permit message.
35      IPermit2.PermitTransferFrom({
36        permitted: IPermit2.TokenPermissions({ token: _token, amount: _amount }),
37        nonce: _nonce,
38        deadline: _deadline
39      }),
40      // The transfer recipient and amount.
41      IPermit2.SignatureTransferDetails({ to: address(this), requestedAmount: _amount }),
42      // The owner of the tokens, which must also be
43      // the signer of the message, otherwise this call
44      // will fail.
45      msg.sender,
46      // The packed signature that was the result of signing
47      // the EIP712 hash of `permit`.
48      _signature
49    );
50  }
51}

Recommendation:

We advise an else branch to be introduced to the referenced if conditional, ensuring that the msg.value is equal-to the input _amount of the Permit2Transfers::takeFromCaller function.

Alleviation:

The native amount is properly validated in a newly introduced else if clause that will yield an InvalidNativeAmount error if an insufficient msg.value has been specified.

As such, we consider this exhibit fully alleviated.

PTS-02M: Inexistent Compatibility of Native Funds

Type	Severity	Location
Logical Fault		Permit2Transfers.sol:L61-L85

Description:

The Permit2Transfers::batchTakeFromCaller function is incompatible with the NATIVE_TOKEN as it will attempt to acquire it from the Permit2 instance instead of validating the call's msg.value.

Impact:

Based on the implementation of ArbitraryExecutionPermit2Adapter::executeWithBatchPermit, which is set as payable, the Permit2Transfers::batchTakeFromCaller is expected to be compatible with native funds. This is not the case in the current implementation by Permit2Transfers.

Example:

Copy
61function batchTakeFromCaller(
62  IPermit2 _permit2,
63  IPermit2.TokenPermissions[] calldata _tokens,
64  uint256 _nonce,
65  uint256 _deadline,
66  bytes calldata _signature
67)
68  internal
69{
70  if (_tokens.length > 0) {
71    _permit2.permitTransferFrom(
72      // The permit message.
73      IPermit2.PermitBatchTransferFrom({ permitted: _tokens, nonce: _nonce, deadline: _deadline }),
74      // The transfer recipients and amounts.
75      _buildTransferDetails(_tokens),
76      // The owner of the tokens, which must also be
77      // the signer of the message, otherwise this call
78      // will fail.
79      msg.sender,
80      // The packed signature that was the result of signing
81      // the EIP712 hash of `permit`.
82      _signature
83    );
84  }
85}

Recommendation:

We advise the code to properly omit the NATIVE_TOKEN from the _tokens array in the PermitBatchTransferFrom struct as well as to skip it when building the transfer details via Permit2Transfers::_buildTransferDetails.

Alleviation:

The Mean Finance team stated that the caller is responsible for ensuring that the native asset is not part of the permit batch, ensuring that a top-level call such as ArbitraryExecutionPermit2Adapter::executeWithBatchPermit would succeed.

We would like to note that this behaviour is contradictory to ArbitraryExecutionPermit2Adapter::executeWithPermit. As such, we advise the Mean Finance team to streamline behaviour across the two implementations.