sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

aman - minPurchaseAmount check wil be bypassed in case of `borrowToken==DAI` #88

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

aman

Medium

minPurchaseAmount check wil be bypassed in case of borrowToken==DAI

Summary

Inside EthenaVault:_executeInstantRedemption, the function is called for instant redemption. This function calls EthenaLib::_sellStakedUSDe to trade assets at the Curve Pool sUSDe:sDAI, then wraps sDAI to DAI. If the borrowToken is not DAI, it trades again to obtain the borrowToken, enforcing the correct trade limit. However, the initial trade from sUSDe to sDAI sets limit=0, which is bypassed when borrowToken=DAI

Vulnerability Detail

for Instant Redemption The Protocol allows user to trade the token on supported Dexes. in case of sUSDe the Protocol allows user to trade the tokens at curve pool to receive the borrow tokens. In case of sUSDe The trading work as follows:

  1. The sUSDe will be first convert into sDAI.
  2. sDAI is wrapped in DAI.
  3. if borrowToken is DAI we return the DAI amount receive, other wise we will convert the DAI into borrow tokens.
function _sellStakedUSDe(
        uint256 sUSDeAmount,
        address borrowToken,
        uint256 minPurchaseAmount,
        bytes memory exchangeData,
        uint16 dexId
    ) internal returns (uint256 borrowedCurrencyAmount) {
        Trade memory sDAITrade = Trade({
            tradeType: TradeType.EXACT_IN_SINGLE,
            sellToken: address(sUSDe),
            buyToken: address(sDAI),
            amount: sUSDeAmount,
            limit: 0, // NOTE: no slippage guard is set here, it is enforced in the second leg
                      // of the trade.
            deadline: block.timestamp,
            exchangeData: abi.encode(CurveV2Adapter.CurveV2SingleData({
                pool: 0x167478921b907422F8E88B43C4Af2B8BEa278d3A,
                fromIndex: 1, // sUSDe
                toIndex: 0 // sDAI
            }))
        });

        (/* */, uint256 sDAIAmount) = sDAITrade._executeTrade(uint16(DexId.CURVE_V2));

        // Unwraps the sDAI to DAI
        uint256 daiAmount = sDAI.redeem(sDAIAmount, address(this), address(this));

@>        if (borrowToken != address(DAI)) {
            Trade memory trade = Trade({
                tradeType: TradeType.EXACT_IN_SINGLE,
                sellToken: address(DAI),
                buyToken: borrowToken,
                amount: daiAmount,
                limit: minPurchaseAmount,
                deadline: block.timestamp,
                exchangeData: exchangeData
            });

            // Trades the unwrapped DAI back to the given token.
            (/* */, borrowedCurrencyAmount) = trade._executeTrade(dexId);
        } else {
            // @audit : the slippage will be also enforced here?? 
  @>        borrowedCurrencyAmount = daiAmount;
        }
    }

As seen in the above code, the limit: minPurchaseAmount is enforced only in the second trade. In the first trade, we accept any value received after trading and wrapping, and returns it without any slippage check.

Impact

The User will receive less token than expected due to missing slippage check in else case.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/staking/protocols/Ethena.sol#L151 https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/staking/protocols/Ethena.sol#L165

Tool used

Manual Review

Recommendation

add Slippage check in else case :

++ require(daiAmount>=minPurchaseAmount);
    borrowedCurrencyAmount = daiAmount;

Duplicate of #18