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

9 stars 8 forks source link

denzi_ - Usage of hardcoded 0 as limit in _sellStakedUSDe() function allows for sandwich opportunities. #109

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

denzi_

High

Usage of hardcoded 0 as limit in _sellStakedUSDe() function allows for sandwich opportunities.

Summary

The Ethena.sol contract has a function called _sellStakedUSDe() which does the following two things

  1. Trade sUSDe into sDAI
  2. Trade DAI into borrowToken after redeeming shares if borrowToken != DAI

The issue is that the limit field is hardcoded to 0 in the 1st Swap, this allows for sandwiching opportunities every time this function is called.

Vulnerability Detail

The function is defined as

function _sellStakedUSDe(
        uint256 sUSDeAmount,
        address borrowToken,
        uint256 minPurchaseAmount,
        bytes memory exchangeData,
        uint16 dexId
    ) internal returns (uint256 borrowedCurrencyAmount)

where minPurchaseAmount is the slippage parameter. This parameter is not used for the first swap and is used later for the 2nd swap.

The vulnerability lies in the the following code where no value for limit is being passed

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
            }))
        });

and the following are the params for the second swap, Note that this trade will only execute only if the borrowToken is not DAI. So only when the 2nd swap happens, then the limit is being enforced.

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
            });

There are a few cases that can occur here.

  1. Attacker sees the minPurchaseAmount being used when the borrowToken is not DAI, front runs the first swap and changes reserves to the point where minPurchaseAmount will still pass the second swap, the first swap is executed followed by the second swap and the attacker backruns to gain their profit.

  2. Attacker sees the function being executed when the borrowToken is DAI, front runs the first swap and changes reserves heavily since no revert can occur due to minPurchaseAmount not being fulfilled (2nd swap will not happen due to no entry into the if block) and after the first swap, the attacker backruns to gain their profit.

Impact

Loss of funds which should not be possible if a limit is also used for the first swap

Code Snippet

_sellStakedUSDe

Tool used

Manual Review

Recommendation

Enforce limit for the first swap

Duplicate of #18