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

Function `MathMasters::mulWadUp` is broken and can return incorrect results.

apex

Summary -

Using increased fuzz testing it was found that the MathMasters::mulWadUp function can return incorrect information.

Vulnerability Details

One such case had the inputs as uint256x = 3323484123583475243233908 and uint256y = 1661742061791737621616955. When used as inputs for MathMasters::mulWadUp the result was 5522773359855710271721681416461. However the correct result is. 5522773359855710271721683078203. Line 56 is noted below. It seems to adjust x under certain conditions, but the logic is not clear. The use of z here is particularly confusing since z is not initialized before this operation.

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

Users of this library can return incorrect information breaking the intented function of the library. Theoretically if a DeFi protocol were to implement this library it could result in funds being lost.

Tools Used

Foundry Fuzz Testing

Recommendations

It is recomended to remove line 56 from the function as described below.

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))
}
}

This new function was ran through an extensive fuzz test of over 100,000,000 inputs and passed each case. It also passed previous input parameters that returned bad information before the function was modified.

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.