Submission Details
Severity:
Incorrect input checking in mulWadUp can lead to calculation errors
Summary
The mulWadUp() function must check that the result of x * y will not overflow, but the condition is incorrect.
Vulnerability Details
The condition if mul(y, gt(x, or(div(not(0), y), x))) is incorrect, as it allows for x * y to overflow.
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))
}
}
Impact
The function will return incorrect calculations, which may lead to loss of funds if used in monetary transactions.
Tools Used
Foundry, Manual review
Proof of Concept
This is a test that should revert, but it doesn't.
function testMulWadUpOverflow1() public {
uint256 y = 100;
uint256 x = (type(uint256).max / y) + 1;
uint256 r = MathMasters.mulWadUp(x, y);
console2.log("r", r);
}
Test passes, confirming that the returned value did overflow without reverting.
forge test --mt testMulWadOverflow
...
[PASS] testMulWadUpOverflow1() (gas: 3775)
Logs:
r 1
Recommended Mitigation
Correct the condition.
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))
}
}
Test it:
function testMulWadUpRevert() public {
uint256 y = 100;
uint256 x = (type(uint256).max / y) + 1;
vm.expectRevert(0xa56044f7);
uint256 r = MathMasters.mulWadUp(x, y);
console2.log("r", r);
}
Test passes, confirming that the function reverted correctly.
forge test --mt testMulWadUpRevert
...
[PASS] testMulWadUpRevert() (gas: 3275)
Updates
Lead Judging Commences
inallhonesty over 1 year ago
Submission Judgement Published Assigned finding tags:
`mulWadUp` has a bad overflow check
Rewards breakdown
nSLOC
46This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.