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

mulWadUp overflow check does not work correctly

kiqo

Summary

On the function MathMasters::mulWadUp, overflow from the multiplication of x * y should result in the function reverting, however, this does not occur.

Vulnerability Details

The overflow check of x * y performed in MathMasters::mulWadUp is:

if mul(y, gt(x, or(div(not(0), y), x)))

This check is flawed and will not detect overflows.

POC

By inserting the following test in MathMasters.t.sol

function testMulWadUpOverFlowCheckFuzz(uint256 x, uint256 y) public {
unchecked {
if (x != 0 && (x * y) / x != y) {
vm.expectRevert();
MathMasters.mulWadUp(x, y);
}
}
}

And running forge test --mt testMulWadUpOverFlowCheckFuzz we get that the function will return an invalid result when the expected behavior would be to revert.

Impact

The function will give an incorrect result instead of reverting, in the event of the multiplication of x * y overflowing.

Tools Used

Foundry fuzzer.

Recommendations

To fix this issue it is recommended that the overflow check of x * y in MathMasters::mulWadUp be equal to the one found in MathMasters::mulWad.

function mulWadUp(uint256 x, uint256 y) internal pure returns (uint256 z) {
/// @solidity memory-safe-assembly
assembly {
// Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
- if mul(y, gt(x, or(div(not(0), y), x))) {
+ if mul(y, gt(x, div(not(0), y))) {
mstore(0x40, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
revert(0x1c, 0x04)
}
if iszero(sub(div(add(z, x), y), 1)) { x := add(x, 1) }
z := add(iszero(iszero(mod(mul(x, y), WAD))), div(mul(x, y), WAD))
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`mulWadUp` has a bad overflow check

Support

FAQs

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