sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ctf_sec - slippage protection is disabled because the trade limit is hardcoded to 0 when Sell residual secondary balance in_executeDynamicTradeExactIn in StragetyUtils.sol #123

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

ctf_sec

medium

slippage protection is disabled because the trade limit is hardcoded to 0 when Sell residual secondary balance in_executeDynamicTradeExactIn in StragetyUtils.sol

Summary

slippage protection is disabled because the trade limit is hardcoded to 0 when Sell residual secondary balance in_executeDynamicTradeExactIn in StragetyUtils.sol

Vulnerability Detail

in the _executeDynamicTradeExactIn in

when we Sell residual secondary balance after unwrap the STETH,

the trade limit is hardcoded to 0, the trade limit is used as a slippage protection.

        // Sell residual secondary balance
        Trade memory trade = Trade(
            params.tradeType,
            sellToken,
            buyToken,
            amount,
            0,
            block.timestamp, // deadline
            params.exchangeData
        );

the trade struct is


struct Trade {
    TradeType tradeType;
    address sellToken;
    address buyToken;
    uint256 amount;
    /// minBuyAmount or maxSellAmount
    uint256 limit;
    uint256 deadline;
    bytes exchangeData;
}

the fifith parameter: limit, in this case is hardcoded to 0,

then the Sell residual secondary balance has no slippage protection and is vulnerable to sandwich attack and front-running from MEV bot.

Impact

then the Sell residual secondary balance has no slippage protection and is vulnerable to sandwich attack and front-running from MEV bot.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/strategy/StrategyUtils.sol#L65-L75

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/interfaces/trading/ITradingModule.sol#L23-L33

Tool used

Manual Review

Recommendation

We recommend the project not hardcode trade limit to 0 to disable the slippage protection. The project can specify the min percentage amount of token received if we do not know the trade limit.

jeffywu commented 2 years ago

When execueTradeWithDynamicSlippage is called the limit parameter is ignored and the slippage is set relative to the oracle. In this case, it is params.oracleSlippagePercent that enforces the slippage, not the limit.