Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

`MathMasters::mulWadUp` overflow check never works

Description

The overflow check in the mulWadUp function of the MathMasters library is flawed due to the or operation. If x contains only one bit different from div(not(0),y), the result of the or operation will be greater than x. Even if div(not(0),y) == x, as x is not strictly superior to x, the condition won't be true.

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))) {
...
}
...
}
}

Impact

Likelihood: High

  • Every call

Impact: High

  • The function will overflow if . This behavior can be exploited by an attacker attempting to manipulate any contract using this library.

Proof of Concept

Foundry PoC added in MathMasters.t.sol
function testMulWadUpOverflow() public {
vm.expectRevert(MathMasters.MathMasters__MulWadFailed.selector);
// will overflow
console2.log(MathMasters.mulWadUp(1e68, 1e68));
}

Recommended Mitigation

Use the same check 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))) {
...
}
...
}
}
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.