sherlock-audit / 2024-03-arrakis-judging

2 stars 2 forks source link

cu5t0mPe0 - Incorrect rounding of precision will cause maxDeviation to fail. #48

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

cu5t0mPe0

medium

Incorrect rounding of precision will cause maxDeviation to fail.

Summary

When calculating price differences, rounding down can render the price difference check invalid, thereby breaking the main invariant.

Vulnerability Detail

In the Arrakis modular, prices are rounded down when calculating slippage, instead of being rounding up.

        if (sqrtSpotPriceX96 <= type(uint128).max) {
            currentPrice = FullMath.mulDiv(
                sqrtSpotPriceX96 * sqrtSpotPriceX96,
                10 ** decimals0,
                2 ** 192
            );
        } else {
            currentPrice = FullMath.mulDiv(
                FullMath.mulDiv(
                    sqrtSpotPriceX96, sqrtSpotPriceX96, 1 << 64
                ),
                10 ** decimals0,
                1 << 128
            );
        }

        uint256 deviation = FullMath.mulDiv(
            currentPrice > oraclePrice
                ? currentPrice - oraclePrice
                : oraclePrice - currentPrice,
            PIPS,
            oraclePrice
        ); 

        if (deviation > maxDeviation_) revert OverMaxDeviation();

If maxDeviation is intended to represent a value of 99.99% (999999), triggering the slippage check would require the price difference between currentPrice and oraclePrice to be nearly twice the oraclePrice.

Assuming oraclePrice is greater than currentPrice, the formula for calculation is: (oraclePrice - currentPrice) / oraclePrice. Due to rounding down, the value of this formula will always be less than PIPS, and therefore always less than 100%, ultimately rendering the price difference percentage check ineffective and thereby breaking the main invariant.

since this behavior breaks the main invariant (invalidating the deviation check), I believe it can be classified as medium risk.

Impact

if (deviation > maxDeviation_) revert OverMaxDeviation(); Check invalid,break the main invariant

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/abstracts/ValantisHOTModule.sol#L472-L478

Tool used

Manual Review

Recommendation

It is recommended to round up when calculating

sherlock-admin3 commented 2 months ago

escalate The issue is somewhat extreme, but when I asked in the PT, the sponsor said: "deviation > maxDeviation_" is the main invariant.

You've deleted an escalation for this issue.

0xjuaan commented 2 months ago

I believe it is low severity, since it is extremely unlikely that a max deviation of 99.99% is going to be used