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

mulWadUp gives incorrect result

kiqo

Summary

Result given by function MathMasters::mulWadUpcan be incorrect depending on the inputs.

Vulnerability Details

In MathMasters::mulWadUp the result of the multiplication (x * y) / WAD rounded up is done by performing:

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

Which returns an incorrect result depending on the input.

POC

By altering the settings of fuzz runs in foundry.tomlin the following fashion:

[fuzz]
- runs = 100
+ runs = 10000
# runs = 1000000 # Use this one for prod
max_test_rejects = 65536
seed = '0x1'
dictionary_weight = 40
include_storage = true
include_push_bytes = true
extra_output = ["storageLayout", "metadata"]

And running forge test --mt testMulWadUpFuzz we find a test case where the expected result is different from what the function returns.

Impact

This causes the behaviour of the function to be different from what is specified in the natspec. Returning wrong results depending on the input.

Tools Used

Foundry fuzzer

Recommendations

To determine whether to round the result of the division up, we simply need to check if the following statement is true:
x * y % WAD != 0

In the case of it being true then we must add 1 to the result of x * y / WAD. This is already done in the function, therefore we must simply remove the line above it which performs an unnecessary 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))
}
}
Updates

Lead Judging Commences

patrickalphac 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.