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

`mulWadUp` function is giving wrong calculation and returning wrong value if rounding is not needed.

naman1729

Summary

  • mulWadUp function is giving wrong calculation when it checks for rounding. It is returning wrong value if rounding is not needed and it is returning the wrong value by adding arbeitrary amount of value in the answer.

Vulnerability Details

  • mulWadUp function is giving wrong calculation when it checks for rounding. It is returning wrong value if rounding is not needed and it is returning the wrong value by adding arbeitrary amount of value in the answer.

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

  • The above pointed check is the reason for the wrong calculation. It is adding 1 to the value of x if the condition is true. This is causing the wrong calculation and returning the wrong value .

Proof of Code

  • This is a fuzzing test to check the functionality of the mulWadUp function in MathMastersTest.t.sol file.

+import {FixedPointMathLib} from "lib/solmate/src/utils/FixedPointMathLib.sol";
​
​
+ function test_MulWadUpFuzz(uint256 x, uint256 y) public pure {
+ if(y == 0 || x <= type(uint256).max / y){
+ assert(MathMasters.mulWadUp(x, y) == FixedPointMathLib.mulWadUp(x, y));
+ }
+ }

  • To run the above test put this command in the terminal.

forge test --mt test_MulWadUpFuzz -vvvv

  • The above test will fail and will give the following error.

[⠒] Compiling...
[⠰] Compiling 2 files with 0.8.23
[⠔] Solc 0.8.23 finished in 2.19s
Compiler run successful!
​
Running 1 test for test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: panic: assertion failed (0x01); counterexample: calldata=0x127211d100000000000000000000000000000000000000000002bfc66f73219d55eb3274000000000000000000000000000000000000000000015fe337b990ceaaf5993b args=[3323484123583475243233908 [3.323e24], 1661742061791737621616955 [1.661e24]]] test_MulWadUpFuzz(uint256,uint256) (runs: 3979, μ: 651, ~: 774)
Traces:
[819] MathMastersTest::test_MulWadUpFuzz(3323484123583475243233908 [3.323e24], 1661742061791737621616955 [1.661e24])
└─ ← panic: assertion failed (0x01)
​
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 277.33ms
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: panic: assertion failed (0x01); counterexample: calldata=0x127211d100000000000000000000000000000000000000000002bfc66f73219d55eb3274000000000000000000000000000000000000000000015fe337b990ceaaf5993b args=[3323484123583475243233908 [3.323e24], 1661742061791737621616955 [1.661e24]]] test_MulWadUpFuzz(uint256,uint256) (runs: 3979, μ: 651, ~: 774)
​
Encountered a total of 1 failing tests, 0 tests succeeded

Impact

  • Wrong calculation by the mulWadUp function

Tools Used

  • Manual Review

Recommendations

  • remove the check from the mulWadUp function.

/// @dev Equivalent to `(x * y) / WAD` rounded up.
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

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.