First Flight #8: Math Master
First Flight #8
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

The `MathMasters::mulWadUp` function has rounding errors in specific scenarios.

Turetos

Summary

The mulWadDown function performs additional computations on the value x, resulting in an incorrect rounding-up outcome.

Vulnerability Details

The mulWadUp function increments the value of x by +1 under the condition that the expression (z + x) / y - 1 evaluates to zero.
Additionally, a bad programming practice is observed where the function signature is stored in the memory position of the free memory pointer during reverts.

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

Protocols using the mathMasters library may generate incorrect outcomes in specific cases.
Here is a test that shows the difference between the round-up function in Solidity versus its optimized Yul version.

function test_MulWadUp(uint128 _a, uint128 _b) public {
uint256 WAD = 1e18;
​
uint a = uint(_a);
uint b = uint(_b);
​
uint mulVal = a * b / WAD;
if (mulVal * WAD < a * b) {
mulVal += 1; //Round up
}
​
uint mulWadUp = MathMasters.mulWadUp(a, b);
​
assertEq(mulVal, mulWadUp);
}
​

Output

Failing tests:
Encountered 1 failing test in test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: assertion failed; counterexample:
calldata=0xe07e166700000000000000000000000000000000fffffffffffffffffffffffffffffffd000000000000000000000000000000007fffffffffffffffffffffffffffffff
args=[340282366920938463463374607431768211453 [3.402e38], 170141183460469231731687303715884105727 [1.701e38]]]
test_MulWadUp(uint128,uint128) (runs: 167, μ: 4338, ~: 4339)

Tools Used

Fuzzing with Foundry, Halmos, manual validation of results with Remix.

Recommendations

Consider removing the line marked below. And modify the value of the memory slot when reverting to 0x00.

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(0x00, 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
Submission Judgement Published
Validated
Assigned finding tags:

`mulWadUp` has an unnecessary line that makes the result wrong for some inputs

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.