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

Function MulWadUp OverFlow Calculation Error

kryptonomousB

Summary

While running a fuzz test at runs of 1000000, an error occurs on the MulWadUp function.

​
Failing tests:
Encountered 1 failing test in test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: Assertion failed. Counterexample: calldata=0xfb6ea94f00000000000000000000000000000000000000000002bfc66f73219d55eb3274000000000000000000000000000000000000000000015fe337b990ceaaf5993b,
args=[3323484123583475243233908, 1661742061791737621616955]] testMulWadUpFuzz(uint256,uint256) (runs: 3979, μ: 913, ~: 1156)

Vulnerability Details

Incorrect Calculation could cause undesired effect as well as potientially loss / locked up of funds.

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

Potiential overflow and possibly security risks.

Tools Used

Forge

Recommendations

While we checked if require(y == 0 || x <= type(uint256).max / y). We need to adjust the operation that checks if x and y could exceed the max value of 'uint256' as well as contain zeros.
change this line:

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

to this line:

if or(iszero(x), iszero(y)){
z := 0
return(0,z)
}
let xy := mul(x, y)
let xyMinus1 := sub(xy, 1)
let divResult := div(xyMinus1, WAD)
​
z:= add(divResult, 1)

if or(iszero(x), iszero(y)) condition checks if either x or y is zero

if either are zero it set z to zero and return (0, z). This will prevent division by zero for ether x or y.

if either are not zero, multiply x * y then sub 1. we then will divide the result by WAD and assign it to Z. This is a simplier more readable structure.

After running the test again all tests pass:

[⠊] Compiling 1 files with 0.8.4
[⠒] Solc 0.8.4 finished in 2.49s
Compiler run successful
​
Running 6 tests for test/MathMasters_Solution.t.sol:MathMastersTest
[PASS] testMulWad() (gas: 647)
[PASS] testMulWadFuzz(uint256,uint256) (runs: 1000000, μ: 570, ~: 658)
[PASS] testMulWadUpFuzz(uint256,uint256) (runs: 1000000, μ: 892, ~: 1146)
[PASS] testSqrt() (gas: 2478)
[PASS] testSqrtFuzzSolmate(uint256) (runs: 1000000, μ: 988, ~: 958)
[PASS] testSqrtFuzzUni(uint256) (runs: 1000000, μ: 12145, ~: 3329)
Test result: ok. 6 passed; 0 failed; finished in 137.10s
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`mulWadUp` has a bad overflow check

Support

FAQs

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