Unsafe downcasting in TWA.sol truncate TWAP price

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/maverick-v1/contracts/libraries/Twa.sol#L18 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/maverick-v1/contracts/libraries/Twa.sol#L12

Vulnerability details

Impact

Unsafe casting operation in TWA.sol truncation price.

Proof of Concept

the pool relies on the TWAP price to function properly, however, the code in TWA.sol sliently downcasting the price, which can truncate the price and affect user's trading unexpectedly.

library Twa {

    using PRBMathSD59x18 for int256;

    function updateValue(IPool.TwaState storage self, int256 value) internal {
        self.twa = self.lastTimestamp == 0 ? int96(value) : int96(getTwa(self));
        self.lastTimestamp = uint48(block.timestamp);
        self.value = int96(value);
    }

    function floor(IPool.TwaState storage self) internal view returns (int32) {
        return int32(PRBMathSD59x18.floor(self.twa) / PRBMathSD59x18.SCALE);
    }

    function getTwa(IPool.TwaState storage self) internal view returns (int256) {

        if (block.timestamp == self.lastTimestamp) {
            return self.twa;
        }

        int256 _timeDiff = int256(block.timestamp - self.lastTimestamp) * PRBMathSD59x18.SCALE;

        int256 _lookback = int256(int16(self.lookback)) * PRBMathSD59x18.SCALE;

        if (_timeDiff >= _lookback) {
            return self.value;
        }

        int256 result = PRBMath.mulDivSigned(self.twa, _lookback - _timeDiff, _lookback) + PRBMath.mulDivSigned(self.value, _timeDiff, _lookback);

        return result;
    }

    function getTwaFloor(IPool.TwaState storage self) internal view returns (int32) {
        return int32(PRBMathSD59x18.floor(getTwa(self)) / PRBMathSD59x18.SCALE);
    }
}

the most damaging truncation is in

function updateValue(IPool.TwaState storage self, int256 value) internal {
    self.twa = self.lastTimestamp == 0 ? int96(value) : int96(getTwa(self));
    self.lastTimestamp = uint48(block.timestamp);
    self.value = int96(value);
}

function floor(IPool.TwaState storage self) internal view returns (int32) {
    return int32(PRBMathSD59x18.floor(self.twa) / PRBMathSD59x18.SCALE);
}

as we can see, the original int256 is truncated into int96 for getTwa price and for floor price truncate from int256 to int32.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project use safeCasting like the project did in other code place.

For example,

In the Pool, the safeCasting is properly used when adding liquidity to used to make sure the code does not truncate value sliently.

binBalanceA += tokenAAmount.toUint128();
binBalanceB += tokenBAmount.toUint128();

[hansfriese]

The twap values are in units of ticks and tick is capped by MAX_TICK = 460540. So the mentioned overflows never happen

[gte620v]

As @hansfriese said. This is not an issue.

[c4-sponsor]

gte620v marked the issue as sponsor disputed

[kirk-baird]

Agreed that these values will not overflow due to the MAX_TICK limit.

[c4-judge]

kirk-baird marked the issue as unsatisfactory: Invalid