Submission Details
Severity:
Incorrect calculations in the function `mulWadUp(uint256 x, uint256 y)`
Vulnerability Details
In function mulWadUp(uint256 x, uint256 y)
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 line of code add 1 to x value if x/y - 1 == 0
POC
Add this test to MathMasters.t.sol and run via forge test --match-test testMulWadUpDelta to see it success.
function testMulWadUpDelta() public {
assertEq(MathMasters.mulWadUp(1e18+1, 1e18), 1e18+2);
assertNotEq(MathMasters.mulWadUp(1e18+1, 1e18), 1e18+1);
assertEq(MathMasters.mulWadUp(2e18+1, 2e18), 4e18+4);
assertNotEq(MathMasters.mulWadUp(2e18+1, 2e18), 4e18+2);
}
Output:
[⠢] Compiling...
No files changed, compilation skipped
Running 2 tests for test/MathMasters.t.sol:MathMastersTest
[PASS] testMulWadUp() (gas: 1110)
[PASS] testMulWadUpFuzz(uint256,uint256) (runs: 100, μ: 959, ~: 1167)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 3.21ms
Ran 1 test suites: 2 tests passed, 0 failed, 0 skipped (2 total tests)
Impact
If x is greater than y by 1 and the result of their multiplication is greater than 1e36
the function will produce an incorrect multiplication result
Tools Used
Forge, manual review.
Recommendations
/// @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 over 1 year ago
Submission Judgement Published Assigned finding tags:
`mulWadUp` has an unnecessary line that makes the result wrong for some inputs
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.