SwapPermit2Adapter Manual Review Findings

SPA-01M: Incorrect Assumption of Input

Type	Severity	Location
Logical Fault		SwapPermit2Adapter.sol:L34, L40, L49

Description:

The _params.swapData payload supplied as part of a Address::functionCallWithValue invocation on the _params.swapper is not sanitized, meaning that it may perform an arbitrary operation rather than an exact-input swap.

Impact:

The _amountIn argument provided by a SwapPermit2Adapter::sellOrderSwap function may be incorrect as has not necessarily been consumed by the _params.swapper interaction.

Example:

Copy
26/// @inheritdoc ISwapPermit2Adapter
27function sellOrderSwap(SellOrderSwapParams calldata _params)
28  public
29  payable
30  checkDeadline(_params.deadline)
31  returns (uint256 _amountIn, uint256 _amountOut)
32{
33  // Take from caller
34  PERMIT2.takeFromCaller(_params.tokenIn, _params.amountIn, _params.nonce, _params.deadline, _params.signature);
35
36  // Max approve token in
37  _params.tokenIn.maxApproveIfNecessary(_params.allowanceTarget);
38
39  // Execute swap
40  _params.swapper.functionCallWithValue(_params.swapData, msg.value);
41
42  // Distribute token out
43  _amountOut = _params.tokenOut.distributeTo(_params.transferOut);
44
45  // Check min amount
46  if (_amountOut < _params.minAmountOut) revert ReceivedTooLittleTokenOut(_amountOut, _params.minAmountOut);
47
48  // Set amount in
49  _amountIn = _params.amountIn;
50}

Recommendation:

We advise the code to either sanitize the _params.swapData passed in if the swapper is known or to re-assess the _amountIn by evaluating whether any remainder of _params.tokenIn lies within the contract akin to SwapPermit2Adapter::buyOrderSwap.

Alleviation:

The Mean Finance team evaluated this exhibit and has opted to acknowledge it, citing that callers can utilize the ArbitraryExecutionPermit2Adapter::executeWithBatchPermit function to process more complex transactions that may require handling leftover funds within the call.

SPA-02M: Potential Re-Entrancy Compromise

Type	Severity	Location
Logical Fault		SwapPermit2Adapter.sol:L80, L86

Description:

A SwapPermit2Adapter::buyOrderSwap interaction is re-entrant during the _amountOut distribution step prior to sending unspent funds to their intended recipient, permitting them to be compromised if _params.transferOut is different than _params.unspentTokenInRecipient.

Impact:

Should the _params.transferOut and _params.unspentTokenInRecipient variables be different, it is possible for a malicious _params.transferOut recipient to re-enter the contract and compromise the funds that were intended for the _params.unspentTokenInRecipient.

Example:

Copy
64function buyOrderSwap(BuyOrderSwapParams calldata _params)
65  public
66  payable
67  checkDeadline(_params.deadline)
68  returns (uint256 _amountIn, uint256 _amountOut)
69{
70  // Take from caller
71  PERMIT2.takeFromCaller(_params.tokenIn, _params.maxAmountIn, _params.nonce, _params.deadline, _params.signature);
72
73  // Max approve token in
74  _params.tokenIn.maxApproveIfNecessary(_params.allowanceTarget);
75
76  // Execute swap
77  _params.swapper.functionCallWithValue(_params.swapData, msg.value);
78
79  // Distribute token out
80  _amountOut = _params.tokenOut.distributeTo(_params.transferOut);
81
82  // Check min amount
83  if (_amountOut < _params.amountOut) revert ReceivedTooLittleTokenOut(_amountOut, _params.amountOut);
84
85  // Send unspent to the set recipient
86  uint256 _unspentTokenIn = _params.tokenIn.sendBalanceOnContractTo(_params.unspentTokenInRecipient);
87
88  // Set amount in
89  _amountIn = _params.maxAmountIn - _unspentTokenIn;
90}

Recommendation:

We advise all function's of the SwapPermit2Adapter to be marked as non-reentrant, disallowing such vulnerabilities from being exploitable.

Alleviation:

The Mean Finance team has proceeded to evaluate the unspent token amount prior to disbursing the _amountOut value, ensuring that a malicious _params.transferOut entry cannot re-enter the contract to compromise the funds meant for the _params.unspentTokenInRecipient.

While a re-entrancy attack could in theory be actuated via the arbitrary _params.swapper call, we consider the likelihood of such an attack low.

As a final note, the Mean Finance team plans to introduce a re-entrancy guard to the overall system in the future once transient storage has been merged to the EVM which would completely rectify all types of re-entrancy vulnerabilities.

SPA-03M: Race-Condition of Permit2 Approvals

Type	Severity	Location
Logical Fault		SwapPermit2Adapter.sol:L27, L34, L64, L71

Description:

The SwapPermit2Adapter will integrate with the Permit2 contract in a non-standard way, consuming a direct permission of a transfer-from operation.

The contract is susceptible to on-chain conditions that would result in the compromise of a user's permitted funds.

To elaborate, the IPermit2::permitTransferFrom function can be invoked by an arbitrary party. As such, a SwapPermit2Adapter::buyOrderSwap function call can be detected by off-chain bots and its permit consumed by a direct call to the Permit2 contract.

As the SwapPermit2Adapter permits arbitrary executions due to the swapper and swapData not being validated, the consumption of the permit can be coupled with a call to the contract that siphons those funds to the malicious party.

Impact:

It is presently possible to siphon any funds supplied as part of a SwapPermit2Adapter::sellOrderSwap or SwapPermit2Adapter::buyOrderSwap function call due to the way these functions attempt to interact with the Permit2 contract of Uniswap.

Example:

Copy
26/// @inheritdoc ISwapPermit2Adapter
27function sellOrderSwap(SellOrderSwapParams calldata _params)
28  public
29  payable
30  checkDeadline(_params.deadline)
31  returns (uint256 _amountIn, uint256 _amountOut)
32{
33  // Take from caller
34  PERMIT2.takeFromCaller(_params.tokenIn, _params.amountIn, _params.nonce, _params.deadline, _params.signature);
35
36  // Max approve token in
37  _params.tokenIn.maxApproveIfNecessary(_params.allowanceTarget);
38
39  // Execute swap
40  _params.swapper.functionCallWithValue(_params.swapData, msg.value);
41
42  // Distribute token out
43  _amountOut = _params.tokenOut.distributeTo(_params.transferOut);
44
45  // Check min amount
46  if (_amountOut < _params.minAmountOut) revert ReceivedTooLittleTokenOut(_amountOut, _params.minAmountOut);
47
48  // Set amount in
49  _amountIn = _params.amountIn;
50}

Recommendation:

We advise the code to revise its Permit2 integration approach. Similarly to the universal router of Uniswap, the Mean Finance team may opt to utilize the IPermit2::transferFrom functions that will attempt to consume an existing approval and supply the "from" argument as the msg.sender.

Alleviation:

After consulting with the Mean Finance team as well as revisiting the Uniswap codebase, we have concluded that the original finding was incorrect given that the Permit2 system of Uniswap performs caller validation at a nested call.

As such, we consider this exhibit nullified.