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

Unnecessary IF statement in `MathMasters::mulWadUp` function

Description: The MathMasters::mulWadUp function has an unnecessary if statement that essentially checks if x == y and then increments x by 1, without providing any context or reason as to why this is necessary. This leads to imprecise returned values.

Impact: High, as it results in imprecise returned values

POC:

function test_mulWadUpOnEqualInput() public pure {
uint256 x = 5e18;
uint256 y = 5e18;
uint256 result = MathMasters.mulWadUp(x, y);
uint256 expected = x * y == 0 ? 0 : (x * y - 1) / 1e18 + 1;
assert(result != expected);
}

Recomendation:

/// @dev Equivalent to `(x * y) / WAD` rounded up.
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))) {
mstore(0x0, 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 an unnecessary line that makes the result wrong for some inputs

Support

FAQs

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