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

Invalid overflow check in `mulWadUp`

Summary

The mulWad and mulWadUp functions both check for overflow in case of multiplication of two numbers before the multiplication is done. However, the condition check in mulWadUp is wrong and may revert even if the multiplication does not overflow.

Vulnerability Details

The condition to check for overflow in mulWadUp is wrong and may revert even if the multiplication does not overflow, which is not expected behaviour. The current condition does not match the require(y == 0 || x <= type(uint256).max / y) . It contains additional or and gt operations which are not needed and also modify the condition in a way that it would revert even if the multiplication does not overflow.

Impact

The impact is high, as it would revert even if the multiplication does not overflow. One example of the issue this may produce is it may affect the protocols using this function as the execution would revert even though it shouldn't, therefore stopping the execution of the protocol.

Tools Used

Foundry fuzz tests

Recommendations

The condition to check for overflow should be fixed to match the one in mulWad function which is correct.

/// @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(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
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.