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

7 stars 5 forks source link

Failure in ETH to stETH Conversion Without User Feedback in EbtcLeverageZapRouter Contract #32

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-badger/blob/main/ebtc-zap-router%2Fsrc%2FZapRouterBase.sol#L34-L41

Vulnerability details

The EbtcLeverageZapRouter contract converts ETH to stETH using the Lido protocol. However, the current implementation of the _depositRawEthIntoLido function does not handle errors that may occur during the conversion process. Specifically, the function uses a low-level call to the Lido contract without checking the return value, which means that if the deposit fails for any reason, the user will not be notified. This lack of feedback can result in user confusion.

Conversation can fail if any of the following occurs:

  1. Stake Limit Reached: If Lido’s staking limit is set and the limit is reached, the deposit will fail.
  2. Staking Paused: If staking is paused in the Lido contract, the deposit will fail.
  3. Insufficient Gas: If there isn’t enough gas to complete the transaction, the deposit will fail.

Impact

Users will not receive any feedback if the deposit to Lido fails, leading to confusion as there's no appropriate error message

Proof of Concept

_depositRawEthIntoLido function that does not handle the potential errors https://github.com/code-423n4/2024-06-badger/blob/main/ebtc-zap-router%2Fsrc%2FZapRouterBase.sol#L34-L41

function _depositRawEthIntoLido(uint256 _initialETH) internal returns (uint256) {
    uint256 _balBefore = stEth.balanceOf(address(this));
    payable(address(stEth)).call{value: _initialETH}("");
    uint256 _deposit = stEth.balanceOf(address(this)) - _balBefore;
    return _deposit;
}

Tools Used

Manual Review

Recommended Mitigation Steps

Use the success variable to check if the call to the Lido contract succeeded. Provide clear and descriptive error messages when the transaction fails.

function _depositRawEthIntoLido(uint256 _initialETH) internal returns (uint256) {
    uint256 _balBefore = stEth.balanceOf(address(this));

    (bool success, ) = payable(address(stEth)).call{value: _initialETH}("");
    require(success, "EbtcZapRouter: ETH to stETH conversion failed");

    uint256 _deposit = stEth.balanceOf(address(this)) - _balBefore;

    return _deposit;
}

Assessed type

Context

c4-judge commented 3 months ago

alex-ppg changed the severity to 3 (High Risk)

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)