code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

Attacker can Steal all eths of WETHRouter.sol through redeem function #278

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/router/WETHRouter.sol#L45-L63

Vulnerability details

Impact

An attacker can Steal eths through redeem function in WETHRouter.sol as you know the contract does the redeem process and redeem user mTokens to ETHs, and as you know we have the function of mint which is the opposite of this and users deposit ETH in order to get Mtoken but the problem is there is no calculation of the amount of ETH should a user get when giving a specific amount of Mtoken so user can redeem with 1 token and drain the whole contract balance also the contract have receive function which is going to receive the eth that accidentally sent to contract and they funds also at risk

Proof of Concept

instance: https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/router/WETHRouter.sol#L41-L64

look at the Redeem Function:


    /// @notice Redeem mToken for ETH
    /// @param mTokenRedeemAmount The amount of mToken to redeem
    /// @param recipient The address to receive the ETH
    function redeem(uint256 mTokenRedeemAmount, address recipient) external {
        IERC20(address(mToken)).safeTransferFrom(
            msg.sender,
            address(this),
            mTokenRedeemAmount
        );

        require(
            mToken.redeem(mTokenRedeemAmount) == 0,
            "WETHRouter: redeem failed"
        );

        weth.withdraw(weth.balanceOf(address(this)));

        (bool success, ) = payable(recipient).call{
            value: address(this).balance
        }("");
        require(success, "WETHRouter: ETH transfer failed");
    }

Tools Used

Vs Code

Recommended Mitigation Steps

Assessed type

Math

0xSorryNotSorry commented 1 year ago

The call stack continues on MErc20's redeem function as below;

    function redeem(uint redeemTokens) override external returns (uint) {
        return redeemInternal(redeemTokens);
    }

Accordingly MErc20.redeem() --> MToken.redeemInternal() --> MToken.redeemFresh() --> MToken.doTransferOut() is called to calculate the underlying tokens with inteterest.

Invalid assumption.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid