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

7 stars 5 forks source link

Unsafe Ether Deposit Using Low-Level call in _depositRawEthIntoLido function #24

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/ebtc-protocol/ebtc-zap-router/blob/40cd479b1ef2606d44cdc36f8fa2c9d84262a794/src/ZapRouterBase.sol#L37

Vulnerability details

Impact

In ZapRouterBase.sol, the _depositRawEthIntoLido function's use of a low-level call introduces significant risks. If the call fails, the function does not revert, which can result in incorrect stETH amounts being returned and leave the contract in an inconsistent state. Additionally, failed transactions are not handled, risking the loss of deposited Ether.

Proof of Concept

Current 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}(""); // ISSUE HERE
    uint256 _deposit = stEth.balanceOf(address(this)) - _balBefore;
    return _deposit;
}

Example Failure Scenario If the call fails, Ether is not deposited into the Lido contract. The function does not revert, potentially returning an incorrect _deposit value. This can lead to subsequent contract logic being based on incorrect assumptions.

Tools Used

vscode

Recommended Mitigation Steps

Recommended code to use:

function _depositRawEthIntoLido(uint256 _initialETH) internal returns (uint256) {
    uint256 _balBefore = stEth.balanceOf(address(this));
    (bool success,) = payable(address(stEth)).call{value: _initialETH}("");
    require(success, "Lido deposit failed");  // Add this required statement
    uint256 _deposit = stEth.balanceOf(address(this)) - _balBefore;
    return _deposit;
}

Assessed type

call/delegatecall

c4-judge commented 3 months ago

alex-ppg marked the issue as unsatisfactory: Insufficient proof

c4-judge commented 3 months ago

alex-ppg changed the severity to 2 (Med Risk)