Submission Details
Severity:
`MathMasters::mulWadUp` function won't revert on overflow
Summary
The MathMasters::mulWadUp function won't revert on overflow due to improper overflow check:
https://github.com/Cyfrin/2024-01-math-master/blob/main/src/MathMasters.sol#L48-L59
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))
}
}
the conditional is not equivalent to require(y == 0 || x <= type(uint256).max / y) as it is doing a bitwise OR operation with x and div(not(0), y).
Vulnerability Details
POC
To reproduce the test case, add the following test to the MathMasters.t.sol file:
function test_MulWadUpRevertsIfOverflow(uint256 x, uint256 y) public {
unchecked {
vm.assume(x != 0 && !((x * y) / x == y));
}
vm.expectRevert();
MathMasters.mulWadUp(x, y);
}
Logs
Running 1 test for test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: call did not revert as expected; counterexample: calldata=0xd04d19600000000000000000000000000000000000000000000000000000000000000002800000000000000000000000000000000000ffffffffffffffffffffffffffff args=[2, 57896044618658097711785492504343953926634997525117140554556420534452894040063 [5.789e76]]] test_MulWadUpRevertsIfOverflow(uint256,uint256) (runs: 0, μ: 0, ~: 0)
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 11.38ms
Impact
The MathMasters::mulWadUp function won't revert when called with values that overflow, and it will output an incorrect result.
Tools Used
Foundry fuzz test.
Recommendations
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 over 2 years 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.