Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buoy3Pool.safetyCheck can underflow #103

Closed
code423n4 opened this issue Jul 7, 2021 · 3 comments
Closed

Buoy3Pool.safetyCheck can underflow #103

code423n4 opened this issue Jul 7, 2021 · 3 comments
Labels
3 (High Risk) bug Something isn't working disagree with severity duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

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
Copy link
Collaborator

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

@kitty-the-kat kitty-the-kat added duplicate This issue or pull request already exists and removed sponsor acknowledged labels Jul 14, 2021
@flabble-gro
Copy link
Collaborator

Duplicate of #6

@flabble-gro flabble-gro marked this as a duplicate of #6 Jul 14, 2021
@ghoul-sol ghoul-sol reopened this Jul 25, 2021
@ghoul-sol
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) bug Something isn't working disagree with severity duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants