ten-on-ten - Un-intended overflow in calculating mid price

[sherlock-admin]

ten-on-ten

medium

Un-intended overflow in calculating mid price

Summary

    function midPrice(Props memory props) internal pure returns (uint256) {
        return (props.max + props.min) / 2;
    }

this function computes average of two uint256, it is possible (however, rare) that both values are less than type(uint256).max and hence average also being less than type(uint256).max , this function would revert instead of returning correct value.

Vulnerability Detail

Function will revert with Panic(17) integer overflow.

Impact

This function is used in ExecuteDepositUtils and SwapUtils, the call execution can revert at those places.

Code Snippet

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/price/Price.sol#L25-L27

Tool used

Manual Review

Recommendation

use https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/Math.sol#L100-L103

[xvi10]

would classify this as low because practically price values should not be configured to reach values that would cause an overflow for this case

[IllIllI000]

@xvi10 can you elaborate on the specific settings that would prevent this?

[xvi10]

sorry the original comment was inaccurate, instead of the configuration it is more that practically this case should not occur

for this case to occur the price per one unit of token would need to be (2**256) / 2 or 5.7896045e+76 USD

[IllIllI000]

I think this is a borderline case, but since it's often handled https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b425a722409fdf4f3e0ec8f42d686f3c0522af19/contracts/utils/math/Math.sol#L105-L108 I'll let Sherlock decide

[hrishibhat]

Agree with the sponsor this can be considered informational