Summary
The function MathMasters::mulWadUp
does not revert for edge case, instead returns zero which could lead to incorrect values parsed when used.
Vulnerability Details
The function MathMasters::mulWadUp
returns zero for the special case where x is a really small number and y is a very large number. The following results below were obtained from the forge inbuilt fuzzer.
[FAIL. Reason: Assertion failed. Counterexample: calldata=0x51913df800000000000000000000000000000000000000000000000000000000000000028000000000000000000000000000000000000000000000000000000000000000, args=[2, 57896044618658097711785492504343953926634992332820282019728792003956564819968 [5.789e76]]] testFuzzMulWadUp(uint256,uint256) (runs: 5, μ: 1013, ~: 1013)
Impact
The function returns zero which could lead to scenarios where MathMasters::mulWadUp
is used for incorrect token transfer amount or incorrect balance.
Tools Used
Forge inbuilt fuzzer
Proof of Concept
Include the following code below in the MathMasters test file.
function mulWadUpHelper(uint256 x, uint256 y) internal pure returns (uint256) {
uint256 product = x * y;
uint256 result = product / MathMasters.WAD;
if (product % MathMasters.WAD > 0) {
result += 1;
}
return result;
}
function testFuzzMulWadUp(uint256 x, uint256 y) public {
uint256 actual = MathMasters.mulWadUp(x, y);
uint256 expected = mulWadUpHelper(x, y);
assertEq(actual, expected, "mulWadUp does not correctly round up");
}
Recommendations
Include the following code after the line z := add(iszero(iszero(mod(mul(x, y), WAD))), div(mul(x, y), WAD))
to catch the edge case where z=0
.
if iszero(z) {
revert(0x1c, 0x04)
}
The function should then look like.
function mulWadUp(uint256 x, uint256 y) internal pure returns (uint256 z) {
assembly {
if mul(y, gt(x, or(div(not(0), y), x))) {
mstore(0x40, 0xbac65e5b)
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))
if iszero(z) {
revert(0x1c, 0x04)
}
}
}