code-423n4 / 2021-06-gro-findings

0 stars 1 forks source link

`Buoy3Pool.safetyCheck` can underflow #103

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The safetyCheck function performs an unsafe subtraction on two uint256 before casting them to int256. The subtraction can underflow and the cast to int256 can either fail and revert the transaction (if greater than type(int256).max), or, fit into an int256 and corrupt the safetyCheck making it always return false.

// _ratio - lastRatio[i] are uint256s and underflows
_ratio = abs(int256(_ratio - lastRatio[i]));

If the lastRatio[i] is even just 1 "wei" less than _ratio, the result will be type(uint256).max and the cast to int256 will fail due to the size limit of signed integers. All functions implementing the safetyCheck will revert and the protocol can become stuck and unusable.

Recommended Mitigation Steps

As only the absolute value is relevant the following code should work without having to cast to int256:

uint256 ratioDiff = _ratio > lastRatio[i] ? _ratio - lastRatio[i] : lastRatio[i] - _ratio;
kitty-the-kat commented 3 years ago

low severity Its not a problem in 0.6.12. No plan to change to 0.8.x

flabble-gro commented 3 years ago

Duplicate of #6

ghoul-sol commented 3 years ago

Duplicate of #6 ergo it's a high risk.