TakeRunSwapAndTransfer Manual Review Findings

TRS-01M: Inexistent Validation of Ether Value

Type	Severity	Location
Language Specific		TakeRunSwapAndTransfer.sol:L39-L48

Description:

The takeRunSwapAndTransfer function will properly fail if insufficient native funds have been sent to the contract (either directly or as part of the call) for the else execution path that attempts to send _parameters.maxAmountIn to the swapper, however, the code will properly execute without an error if native funds have been sent in the takeRunSwapAndTransfer call but the tokenIn is not the PROTOCOL_TOKEN, a case that should be prohibited.

Impact:

In the current implementation, it is possible for native funds to be lost and stolen either deliberately or accidentally by other parties as any native funds sent to the takeRunSwapAndTransfer call without the PROTOCOL_TOKEN specified will remain in the contract until claimed by another native-fund using transaction.

Example:

Copy
27/**
28 * @notice Takes tokens from the caller, and executes a swap with the given swapper. The swap itself
29 *         should include a transfer, or the swapped tokens will be left in the contract
30 * @dev This function can only be executed with swappers that are allowlisted
31 * @param _parameters The parameters for the swap
32 */
33function takeRunSwapAndTransfer(TakeRunSwapAndTransferParams calldata _parameters)
34  public
35  payable
36  virtual
37  onlyAllowlisted(_parameters.swapper)
38{
39  if (_parameters.tokenIn != PROTOCOL_TOKEN) {
40    _takeFromMsgSender(IERC20(_parameters.tokenIn), _parameters.maxAmountIn);
41    _maxApproveSpenderIfNeeded(
42      IERC20(_parameters.tokenIn),
43      _parameters.allowanceTarget,
44      _parameters.swapper == _parameters.allowanceTarget, // If target is a swapper, then it's ok as allowance target
45      _parameters.maxAmountIn
46    );
47    _executeSwap(_parameters.swapper, _parameters.swapData, 0);
48  } else {
49    _executeSwap(_parameters.swapper, _parameters.swapData, _parameters.maxAmountIn);
50  }
51  if (_parameters.checkUnspentTokensIn) {
52    _sendBalanceOnContractToRecipient(_parameters.tokenIn, msg.sender);
53  }
54  _sendBalanceOnContractToRecipient(_parameters.tokenOut, _parameters.recipient);
55}

Recommendation:

We advise the contract to properly validate that no native funds were sent if _parameters.tokenIn != PROTOCOL_TOKEN.

Alleviation:

The Mean Finance team stated that due to the contract's intention to be utilized with multi-calls, it is impossible to deduce whether the ether natively sent was meant for this particular function or whether it was accidentally sent. As a result, they are unable to protect against accidental transfers. In this case, we consider the exhibit as an acknowledged issue that cannot be alleviated in the codebase.