First Flight #8: Math Master
First Flight #8
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Calculation result overflow in MathMasters::mulWad, might lead to rounding error

0xlouistsai

Summary

In the documentation of mulWad function in MathMasters, the result of the calculation should be rounded down for whatever x and y pair.
The result of mulWad is equivalent to the following function:

function mulWadCorrectVersion(uint256 x, uint256 y) public pure returns(uint256 z) {
z = x * y / 1e18;
}

Solidity will automatically do the rounding for the calculation. However, there exists some x and y such that the result is inconsistent to mulWadCorrectVersion function.

Vulnerability Details

Add the following test in MathMasters.t.sol

function testMulWadException() public {
uint256 x = 2;
uint256 y = 57896044618658097711785492504343953926634997525117140554556420534452894040063;
console2.log(x*y);
assertEq(MathMasters.mulWad(2, 57896044618658097711785492504343953926634997525117140554556420534452894040063), x * y / 1e18)
}

The error message shows that there is arithmetic overflow, which means the data validation in the previous step is invalid:

function mulWad(uint256 x, uint256 y) internal pure returns (uint256 z) {
// @solidity memory-safe-assembly
assembly {
@> // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
// @audit-issue the overflow detection method invalid
if mul(y, gt(x, div(not(0), y))) {
mstore(0x40, 0xbac65e5b)
revert(0x1c, 0x04)
}
z := div(mul(x, y), WAD)
}
}

Modify the test case testMulWadFuzz in MathMaster.t.sol:

function testMulWadFuzz(uint256 x, uint256 y) public pure {
// Ignore cases where x * y overflows.
// unchecked {
// if (x != 0 && (x * y) / x != y) return;
// }
unchecked {
if (y==0||x<=type(uint256).max / y) return;
}
​
assert(MathMasters.mulWad(x, y) == (x * y) / 1e18);
}

After the unchecked statement that filter the possibility of overflow in multiplication, the mulWad function should always success, but the test fails for certain test case.

Impact

Inconsistency in rounding might lead to severe consequence and multiple historic attack event originates in these types of vulnerability. The most famous one including the Balancer V2 Incident.

Tools Used

Foundry

Recommendations

Update the condition of overflow, and compare with the mulWadCorrectVersion using fuzzing tools.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.