code-423n4 / 2024-06-badger-findings

7 stars 5 forks source link

`_depositRawEthIntoLido` doesn't check for after - before balance, can lead to first depositor vulnerability #16

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-badger/blob/9173558ee1ac8a78a7ae0a39b97b50ff0dd9e0f8/ebtc-zap-router/src/ZapRouterBase.sol#L34-L41

Vulnerability details

Summary

_depositRawEthIntoLido doesn't check for after - before balance

Vulnerability Details

In the function _depositRawEthIntoLido from the comment // check before-after balances for 1-wei corner case , we can infer that the function is supposed to check for the balance of token before and after the deposit of raw Eth in Lido.

Let us consider at this stage we have 10 stEth as current balance and _initialETH = 5, and if we simulate the _depositRawEthIntoLido with the assumed valued we get

_initialETH = 5
 _balBefore = 10 //gets current stEth balance of contract
payable(address(stEth)).call{value: 5}("") // transfers 5stEth
//current balance = 10 + 5 = 15 stEth
_deposit= 15-10//gets amount of stEth deposited 

As we can see the function just checks balance of the contract before the transfer and the amount deposited to the contract. And doesn't checks the balance of contract after and before the transfer.

Impact

Function can be susceptible to first depositor vulnerability or 1-wei corner case

Proof of code

    function _depositRawEthIntoLido(uint256 _initialETH) internal returns (uint256) {
        // check before-after balances for 1-wei corner case
        uint256 _balBefore = stEth.balanceOf(address(this));
        // TODO call submit() with a referral?
        payable(address(stEth)).call{value: _initialETH}("");
        uint256 _deposit = stEth.balanceOf(address(this)) - _balBefore;
        return _deposit;
    }

https://github.com/code-423n4/2024-06-badger/blob/9173558ee1ac8a78a7ae0a39b97b50ff0dd9e0f8/ebtc-zap-router/src/ZapRouterBase.sol#L34-L41

Tools Used

Manual review

Recommended Mitigation

Check the after and before balance of the contract after transfers.

Assessed type

Context

GalloDaSballo commented 4 months ago

We do check for errors, but it happens later The Router has no assets so you will not be able to add tokens if there's a rounding error, using a more appropriate amount prevents this

alex-ppg commented 3 months ago

The Warden describes a case which does not hold true; the present code measures balances before and after the stETH deposit operation. As such, the code presently behaves as expected and no vulnerability arises from this submission.

c4-judge commented 3 months ago

alex-ppg marked the issue as unsatisfactory: Invalid