Omniscia Mean Finance Audit

ArbitraryExecutionPermit2Adapter Manual Review Findings

ArbitraryExecutionPermit2Adapter Manual Review Findings

AEP-01M: Potential Increase of Functionality

TypeSeverityLocation
Logical FaultArbitraryExecutionPermit2Adapter.sol:L86-L92

Description:

Given that the ArbitraryExecutionPermit2Adapter contract is meant to acquire the necessary funds from the caller and perform a sequence of calls with them, it is expected that any leftover funds are to be refunded to the caller.

The current code relies on a properly structured _transferOut entry that may ultimately not be supplied by the function's callers, leading to leftover funds being up for grabs by any party.

Impact:

While the function will behave as expected if used properly, we advise minimizing its reliance on input arguments to ensure that callers can interact with the contract in a safety-first manner rather than a flexibility-first approach.

Example:

src/base/ArbitraryExecutionPermit2Adapter.sol
25/// @inheritdoc IArbitraryExecutionPermit2Adapter
26function executeWithPermit(
27  SinglePermit calldata _permit,
28  AllowanceTarget[] calldata _allowanceTargets,
29  ContractCall[] calldata _contractCalls,
30  TransferOut[] calldata _transferOut,
31  uint256 _deadline
32)
33  external
34  payable
35  checkDeadline(_deadline)
36  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
37{
38  PERMIT2.takeFromCaller(_permit.token, _permit.amount, _permit.nonce, _deadline, _permit.signature);
39  return _approveExecuteAndTransfer(_allowanceTargets, _contractCalls, _transferOut);
40}
41

42/// @inheritdoc IArbitraryExecutionPermit2Adapter
43function executeWithBatchPermit(
44  BatchPermit calldata _batchPermit,
45  AllowanceTarget[] calldata _allowanceTargets,
46  ContractCall[] calldata _contractCalls,
47  TransferOut[] calldata _transferOut,
48  uint256 _deadline
49)
50  external
51  payable
52  checkDeadline(_deadline)
53  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
54{
55  PERMIT2.batchTakeFromCaller(_batchPermit.tokens, _batchPermit.nonce, _deadline, _batchPermit.signature);
56  return _approveExecuteAndTransfer(_allowanceTargets, _contractCalls, _transferOut);
57}
58

59function _approveExecuteAndTransfer(
60  AllowanceTarget[] calldata _allowanceTargets,
61  ContractCall[] calldata _contractCalls,
62  TransferOut[] calldata _transferOut
63)
64  internal
65  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
66{
67  // Approve targets
68  for (uint256 i; i < _allowanceTargets.length;) {
69    IERC20(_allowanceTargets[i].token).maxApprove(_allowanceTargets[i].allowanceTarget);
70    unchecked {
71      ++i;
72    }
73  }
74

75  // Call contracts
76  _executionResults = new bytes[](_contractCalls.length);
77  for (uint256 i; i < _contractCalls.length;) {
78    _executionResults[i] =
79      _contractCalls[i].target.functionCallWithValue(_contractCalls[i].data, _contractCalls[i].value);
80    unchecked {
81      ++i;
82    }
83  }
84

85  // Distribute tokens
86  _tokenBalances = new uint256[](_transferOut.length);
87  for (uint256 i; i < _transferOut.length;) {
88    _tokenBalances[i] = _transferOut[i].token.distributeTo(_transferOut[i].distribution);
89    unchecked {
90      ++i;
91    }
92  }
93}

Recommendation:

We advise the code to either build an automatic _transferOut list from the supplied input tokens of an arbitrary call, or to validate that the _transferOut array contains at least all input tokens of an arbitrary call.

We consider either of the two approaches sufficient for alleviating this exhibit.

Alleviation:

The Mean Finance team evaluated this exhibit and opted to acknowledge it as they wish to leave the onus of proper calls on the caller.

AEP-02M: Lingering Token Approvals

TypeSeverityLocation
Logical FaultArbitraryExecutionPermit2Adapter.sol:L69

Description:

The ArbitraryExecutionPermit2Adapter contract is meant to act as an intermediary in a variety of interactions with other protocols. In the current implementation, the contract will approve the maximum amount possible to a potential call target.

As such, a non-zero approval may remain after the call. This can be exploited by scammers and other similar hijacking attacks by approving a malicious contract and attempt to inject this contract in an unsuspecting user's call-chain even if they never approved it explicitly during their invocation of the function.

Impact:

The current implementation of ArbitraryExecutionPermit2Adapter::_approveExecuteAndTransfer is not atomic, permitting it to be exploited in numerous ways including re-entrancies as well as secondary-call approval attacks.

Example:

src/base/ArbitraryExecutionPermit2Adapter.sol
59function _approveExecuteAndTransfer(
60  AllowanceTarget[] calldata _allowanceTargets,
61  ContractCall[] calldata _contractCalls,
62  TransferOut[] calldata _transferOut
63)
64  internal
65  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
66{
67  // Approve targets
68  for (uint256 i; i < _allowanceTargets.length;) {
69    IERC20(_allowanceTargets[i].token).maxApprove(_allowanceTargets[i].allowanceTarget);
70    unchecked {
71      ++i;
72    }
73  }
74

75  // Call contracts
76  _executionResults = new bytes[](_contractCalls.length);
77  for (uint256 i; i < _contractCalls.length;) {
78    _executionResults[i] =
79      _contractCalls[i].target.functionCallWithValue(_contractCalls[i].data, _contractCalls[i].value);
80    unchecked {
81      ++i;
82    }
83  }
84

85  // Distribute tokens
86  _tokenBalances = new uint256[](_transferOut.length);
87  for (uint256 i; i < _transferOut.length;) {
88    _tokenBalances[i] = _transferOut[i].token.distributeTo(_transferOut[i].distribution);
89    unchecked {
90      ++i;
91    }
92  }
93}

Recommendation:

We advise all approvals performed before a function call to be zeroed out after the calls have been performed. Additionally, we advise the ArbitraryExecutionPermit2Adapter::_approveExecuteAndTransfer function to be set as non-reentrant given that each Token::distributeTo call can become re-entrant and could result in the loss of ensuing token funds.

Alleviation:

The Mean Finance team introduced a code segment right after the external calls are processed in the ArbitraryExecutionPermit2Adapter::_approveExecuteAndTransfer function that resets the allowances to 1, essentially "erasing" them after the external calls have concluded. As such, we consider this exhibit fully alleviated given that no lingering approvals will remain post-execution of a Permit2 arbitrary execution call.

AEP-03M: Race-Condition of Permit2 Approvals

TypeSeverityLocation
Logical FaultArbitraryExecutionPermit2Adapter.sol:L38-L39, L55-L56

Description:

The ArbitraryExecutionPermit2Adapter 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, an ArbitraryExecutionPermit2Adapter::executeWithPermit function call can be detected by off-chain bots and its permit consumed by a direct call to the Permit2 contract.

As the ArbitraryExecutionPermit2Adapter permits arbitrary executions, 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 an ArbitraryExecutionPermit2Adapter::executeWithPermit or ArbitraryExecutionPermit2Adapter::executeWithBatchPermit function call due to the way these functions attempt to interact with the Permit2 contract of Uniswap.

Example:

src/base/ArbitraryExecutionPermit2Adapter.sol
25/// @inheritdoc IArbitraryExecutionPermit2Adapter
26function executeWithPermit(
27  SinglePermit calldata _permit,
28  AllowanceTarget[] calldata _allowanceTargets,
29  ContractCall[] calldata _contractCalls,
30  TransferOut[] calldata _transferOut,
31  uint256 _deadline
32)
33  external
34  payable
35  checkDeadline(_deadline)
36  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
37{
38  PERMIT2.takeFromCaller(_permit.token, _permit.amount, _permit.nonce, _deadline, _permit.signature);
39  return _approveExecuteAndTransfer(_allowanceTargets, _contractCalls, _transferOut);
40}
41

42/// @inheritdoc IArbitraryExecutionPermit2Adapter
43function executeWithBatchPermit(
44  BatchPermit calldata _batchPermit,
45  AllowanceTarget[] calldata _allowanceTargets,
46  ContractCall[] calldata _contractCalls,
47  TransferOut[] calldata _transferOut,
48  uint256 _deadline
49)
50  external
51  payable
52  checkDeadline(_deadline)
53  returns (bytes[] memory _executionResults, uint256[] memory _tokenBalances)
54{
55  PERMIT2.batchTakeFromCaller(_batchPermit.tokens, _batchPermit.nonce, _deadline, _batchPermit.signature);
56  return _approveExecuteAndTransfer(_allowanceTargets, _contractCalls, _transferOut);
57}

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.

Prev

Token.sol (TNE-S)

Next

Permit2Transfers.sol (PTS-M)

On this page