Unchecked return value on call in the _convertRawEthToStETH function on the ZapRouterBase contract

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

The external function named openCdpWithEth calls the function named _convertRawEthToStETH, which calls the function _depositRawEthIntoLido does not check the return value of the external call to the stEth contract. This can lead to a situation where the call to the stEth contract fails silently, and the function continues execution without any indication of failure. This could result in a discrepancy in the accounting of stEth balances, potentially leading to financial loss.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

    function _convertRawEthToStETH(uint256 _initialETH) internal returns (uint256) {
        require(msg.value == _initialETH, "EbtcZapRouter: Incorrect ETH amount");
        return _depositRawEthIntoLido(_initialETH);
    }

In the above function, the external call is made using _depositRawEthIntoLido in call{value: _initialETH}("") without checking the success of the operation.

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

        uint256 _collVal = _convertRawEthToStETH(_ethMarginBalance);

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

    function openCdpWithEth(
        uint256 _debt,
        bytes32 _upperHint,
        bytes32 _lowerHint,
        uint256 _stEthLoanAmount,
        uint256 _ethMarginBalance,
        uint256 _stEthDepositAmount,
        bytes calldata _positionManagerPermit,
        TradeData calldata _tradeData
    ) external payable returns (bytes32 cdpId) {
        uint256 _collVal = _convertRawEthToStETH(_ethMarginBalance);

        cdpId = _openCdp(
            _debt,
            _upperHint,
            _lowerHint,
            _stEthLoanAmount,
            _collVal,
            _stEthDepositAmount,
            _positionManagerPermit,
            _tradeData
        );

        emit ZapOperationEthVariant(
            cdpId,
            EthVariantZapOperationType.OpenCdp,
            true,
            NATIVE_ETH_ADDRESS,
            _ethMarginBalance,
            _collVal,
            msg.sender
        );
    }

Tools Used

Manual review.

Recommended Mitigation Steps

To mitigate this issue,

1. The openCdpWithEth function needs to validate the _convertRawEthToStETH value like so:

   uint256 _collVal = _convertRawEthToStETH(_ethMarginBalance);
   require(_collVal > 0, "Invalid collateral value");

2. The _depositRawEthIntoLido function should be modified to check the return value of the external call and revert if the call fails. Here is the recommended change:

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

By adding the require(success, "Deposit to Lido failed"); statement, the function will revert if the external call to stEth fails, ensuring that the transaction does not proceed with an incorrect stEth balance.

Assessed type

call/delegatecall

[c4-judge]

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

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

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