Re-entrancy in mevETH deposit

Problem

Reentrancy in MevEth.deposit(uint256,address) (src/MevEth.sol#254-279):
        External calls:
        - WETH.transferFrom(msg.sender,address(this),assets) (src/MevEth.sol#255)
        - WETH.withdraw(assets) (src/MevEth.sol#257)
        State variables written after the call(s):
        - assetRebase.elastic += assets (src/MevEth.sol#273)
        - assetRebase.base += shares (src/MevEth.sol#274)
        - _mint(receiver,shares) (src/MevEth.sol#276)
                - balanceOf[to] += amount (lib/solmate/src/tokens/ERC20.sol#175)
        - _mint(receiver,shares) (src/MevEth.sol#276)
                - totalSupply += amount (lib/solmate/src/tokens/ERC20.sol#170)

Solution

Write state vars before calls

    /// @param assets The amount of WETH which should be deposited
    /// @param receiver The address user whom should recieve the mevEth out
    /// @return shares The amount of shares minted
    function deposit(uint256 assets, address receiver) external stakingUnpaused returns (uint256 shares) {
        uint256 balance = address(this).balance;
        if (assetRebase.elastic == 0 || assetRebase.base == 0) {
            shares = assets;
        } else {
            shares = (assets * assetRebase.elastic) / assetRebase.base;
        }

        if (assetRebase.base + shares < 1000) {
            revert MevEthErrors.DepositTooSmall();
        }

        assetRebase.elastic += assets;
        assetRebase.base += shares;

        _mint(receiver, shares);

        WETH.transferFrom(msg.sender, address(this), assets);
        WETH.withdraw(assets);
        // Not really neccessary, but protects against malicious WETH implementations
        if (balance + assets != address(this).balance) {
            revert MevEthErrors.DepositFailed();
        }

        emit Deposit(msg.sender, receiver, assets, shares);
    }

manifoldfinance / mevETH2

Re-entrancy in mevETH deposit #83

Problem

Solution