c4-bot-4 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L480 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L434 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L366 https://github.com/code-423n4/2024-03-abracadabra-money/blob/1f4693fdbf33e9ad28132643e2d6f7635834c6c6/src/mimswap/periphery/Router.sol#L276

Vulnerability details

Impact

When a function in a Solidity contract interacts with Ether (ETH), such as receiving or sending ETH, it's customary to mark the Address as payable. Marking an Address as payable indicates that it can receive Ether as part of a transaction.

Proof of Concept

A. In the provided function swapTokensForETH, the contract is interacting with ETH by performing the following actions:

Withdrawing ETH from the WETH contract (weth.withdraw(amountOut)).
Transferring ETH to the recipient (to.safeTransferETH(amountOut)).

Since the function is involved in transferring ETH, it should mark to as payable to ensure it can receive Ether and handle such transactions correctly.

B. In the provided sellBaseTokensForETH function, the contract is interacting with Ether (ETH) by withdrawing ETH from the WETH contract (weth.withdraw(amountOut)) and transferring ETH to the recipient (to.safeTransferETH(amountOut)). Since the function is involved in transferring ETH, it should mark to as payable to ensure it can receive Ether and handle such transactions correctly.

C. In the sellQuoteTokensForETH function, Ether (ETH) is withdrawn from the WETH contract and transferred to the recipient. Since the function involves handling ETH, it should mark to as payable to ensure it can receive Ether and handle such transactions correctly.

D. The removeLiquidityETH function removes liquidity from a liquidity pool (LP) and transfers Ether and tokens to the specified recipient (to).

Handling ETH and token transfers: The function performs sellshares within the LP to obtain Ether or tokens depending on the type of LP. If the LP's base token is WETH. After the selling, it withdraws Ether from the WETH contract and transfers it to the recipient. Additionally, it transfers tokens to the recipient.

Tools Used

Manual code analysis

Recommended Mitigation Steps

To fix this potential issue, you should add payable to the address input : A.

function swapTokensForETH(
    address payable to,
    uint256 amountIn,
    address[] calldata path,
    uint256 directions,
    uint256 minimumOut,
    uint256 deadline
) external  ensureDeadline(deadline) returns (uint256 amountOut) {
    }

B.

function sellBaseTokensForETH(
    address lp,
    address payable to,
    uint256 amountIn,
    uint256 minimumOut,
    uint256 deadline
) external ensureDeadline(deadline) returns (uint256 amountOut) {

}

C.

function sellQuoteTokensForETH(
    address lp,
    address payable to,
    uint256 amountIn,
    uint256 minimumOut,
    uint256 deadline
) external ensureDeadline(deadline) returns (uint256 amountOut) {

}

D.

function removeLiquidityETH(
    address lp,
    address payable to,
    uint256 sharesIn,
    uint256 minimumETHAmount,
    uint256 minimumTokenAmount,
    uint256 deadline
) external returns (uint256 ethAmountOut, uint256 tokenAmountOut) {

}

By adding payable to the Address, you explicitly indicate that it can receive Ether, ensuring proper handling of ETH transactions.