Incorrect overflow protection can lead to incorrect pricing from adapters

Summary

In the exponential and logarithmic price adapter protection against overflow of expArgument is done by checking that it is not > type(uint256).max but expArgument is an int256. expArgument could therefore be cast to int256 from a unit256 that is > type(int256).max.

Vulnerability Detail

In getPrice() in both the logarithmic and exponential price adapter we have this issue


        if (timeBucket > type(uint256).max / timeCoefficient) {
            return _getBoundaryPrice(isDecreasing, maxPrice, minPrice);
        }
        int256 expArgument = int256(timeCoefficient * timeBucket)

and

        if (timeBucket > type(uint256).max / timeCoefficient) {
            return _getBoundaryPrice(isDecreasing, maxPrice, minPrice);
        }
        int256 lnArgument = int256(timeBucket * timeCoefficient);

If timeBukcet*timeCoefficient> type(uint256).max/2 we will "wrap" around into negative numbers going from -1 to type(int256).min.

Impact

Unexpected price changes. Here are some of the potential price changes.

In the exponential adapter the price starts going in the opposite direction since the argument can be negative.

The logarithmic adapter is more unpredictable. Could either revert and block the auction or jump to min/max depending on size of the lnArgument variable.

uint256 lnExpression = uint256(FixedPointMathLib.lnWad(lnArgument + WAD));

Code Snippet

https://github.com/sherlock-audit/2023-06-Index/blob/main/index-protocol/contracts/protocol/integration/auction-price/BoundedStepwiseLogarithmicPriceAdapter.sol#L58-L61

https://github.com/sherlock-audit/2023-06-Index/blob/main/index-protocol/contracts/protocol/integration/auction-price/BoundedStepwiseExponentialPriceAdapter.sol#L58-L61

Tool used

Manual Review

Recommendation

Change

        if (timeBucket > type(uint256).max / timeCoefficient) {
            return _getBoundaryPrice(isDecreasing, maxPrice, minPrice);
        }

        if (timeBucket > type(int256).max / timeCoefficient) {
            return _getBoundaryPrice(isDecreasing, maxPrice, minPrice);
        }

Duplicate of #1

sherlock-audit / 2023-06-Index-judging

qandisa - Incorrect overflow protection can lead to incorrect pricing from adapters #72