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

0 stars 1 forks source link

`Exposure.sortVaultsByDelta` can underflow #99

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The sortVaultsByDelta 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 correct sorting of the vaults that follows.

int256 delta = int256(
  // this is still doing unsafe uint256 subtraction
    unifiedAssets[i] - unifiedTotalAssets.mul(targetPercents[i]).div(PERCENTAGE_DECIMAL_FACTOR)
);

Example values:

On i = 0, the computation would be:

unifiedAssets[i] - unifiedTotalAssets.mul(targetPercents[i]).div(PERCENTAGE_DECIMAL_FACTOR) = 30_000 * 1e18 - (100_000 * 1e18 * 40%) = 30_000 * 1e18 - 40_000 * 1e18 = 2^256 - 10_000 * 1e18 // overflow

The transaction would then fail because the result is greater than typeof(int256).max.

Recommended Mitigation Steps

Check each term individually if they fit into an int256 and cast them to int256 before the subtraction.

kitty-the-kat commented 3 years ago

It is no problem in 0.6.12. No plan to change to 0.8.x.

ghoul-sol commented 3 years ago

For 0.6.12 it works. Invalid.