sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

hansfriese - Possible rounding error during `1 / oraclePrice` calculation in `TradingUtils._getLimitAmount()`. #111

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

hansfriese

medium

Possible rounding error during 1 / oraclePrice calculation in TradingUtils._getLimitAmount().

Summary

Possible rounding error during 1 / oraclePrice calculation in TradingUtils._getLimitAmount().

Vulnerability Detail

Possible rounding error during 1 / oraclePrice calculation in TradingUtils._getLimitAmount().

Currently, it uses the oracleDecimals to increase the precision here.

// For exact out trades, we need to invert the oracle price (1 / oraclePrice)
// We increase the precision before we divide because oraclePrice is in
// oracle decimals
oraclePrice = (oracleDecimals * oracleDecimals) / oraclePrice;
// For exact out trades, limitAmount is the max amount of sellToken the DEX can
// pull from the contract
limitAmount =
    ((oraclePrice + 
        ((oraclePrice * uint256(slippageLimit)) /
            Constants.SLIPPAGE_LIMIT_PRECISION)) * amount) / 
    oracleDecimals;

But as we can see here, some tokens have low decimals and there would be a rounding error during calculation.

Impact

The limitAmount might be calculated wrongly due to the rounding error.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol#L192

Tool used

Manual Review

Recommendation

I think we should use 1e18 instead of oracleDecimals like below.

// For exact out trades, we need to invert the oracle price (1 / oraclePrice)
// We increase the precision before we divide because oraclePrice is in
// oracle decimals
oraclePrice = (oracleDecimals * 1e18) / oraclePrice; //++++++++++++++++++++++++++++++++++++++++++
// For exact out trades, limitAmount is the max amount of sellToken the DEX can
// pull from the contract
limitAmount =
    ((oraclePrice + 
        ((oraclePrice * uint256(slippageLimit)) /
            Constants.SLIPPAGE_LIMIT_PRECISION)) * amount) / 
    1e18; //++++++++++++++++++++++++++++++++++++++++++
jeffywu commented 1 year ago

Oracle decimals is in 1e18 at this point