First Flight #8: Math Master

Summary

mulWadUp returns unexpected, incorrect value.

Vulnerability Details

When x = 3323484123583475243233908 [3.323e24], y = 1661742061791737621616955 [1.661e24], the function returns an incorrect value as seen in the fuzzing logs:

[FAIL. Reason: assertion failed] testMulWadUp2() (gas: 18283)
Logs:
5522773359855710271721683078203 5522773359855710271721681416461

Error: a == b not satisfied [uint]

    Left: 5522773359855710271721683078203 (result)


    Right: 5522773359855710271721681416461 (expected)

Impact

Some calculations will be incorrect, resulting in possible critical bugs for whichever contract uses this library.

Tools Used

Forge fuzzing with 1M runs.

Recommendations

Line 56 includes:
if iszero(sub(div(add(z, x), y), 1)) { x := add(x, 1) }

I could not figure out why that line exists, because mulWadUp can be correctly calculated with Line 57 alone:

z := add(iszero(iszero(mod(mul(x, y), WAD))), div(mul(x, y), WAD))

That says z is x * y / WAD if x * y is a multiple of WAD, else z is (x * y / WAD) + 1 meaning it's not a multiple and we round up.
So to fix this bug you remove line 56.