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

Due to an incorrect memory address handling in `MathMasters.sol::mulWad()` and `MathMasters.sol::mulWadUp()` functions, a failed function call does not revert with the intended error

stefanlatinovic

Summary

When a function call fails due to overflow, it does not revert with the intended error. Instead, it reverts without providing a reason.

Vulnerability Details

The information regarding the reverted error is stored at the memory location of the free memory pointer (0x40), but the functions revert with the data stored at the memory location 0x1c.

It's important to note that there is no need to override the free memory pointer in this case, and it is not being updated in a secure manner. This overriding will result in very expensive memory expansion, and If the function wouldn't revert, there would be a possibility of encountering "Out of Gas" error.

Impact

When an overflow occurs, the function reverts without providing an intended error.

Proof of Concept (PoC)

We can check whether the function call will revert with the intended error.

Add the following test in MathMasters.t.sol:

function test_MulWadShouldRevertWithIntendedError(uint256 x, uint256 y) public {
vm.assume(y != 0 && x > type(uint256).max / y); // Input values that will result in overflow
​
vm.expectRevert(0xbac65e5b); // intended revert error
MathMasters.mulWad(x, y);
}

Run a test with forge test --mt test_MulWadShouldRevertWithIntendedError.

We should see an output similar to this in the terminal:

Running 1 test for test/MathMasters.t.sol:MathMastersTest
[FAIL. Reason: Error != expected error: != 0xbac65e5b; counterexample: calldata=0xc680b3700000000000000000000000000000000000000000000000000000000000000002800000000000000000000000000000000001ffffffffffffffffffffffffffff args=[2, 57896044618658097711785492504343953926635002717413999089384049064949223260159 [5.789e76]]] test_MulWadShouldRevertWithIntendedError(uint256,uint256) (runs: 0, μ: 0, ~: 0)
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 13.95ms

This test has failed because the function call did revert, but not with an error that was expected in the test.

Tools Used

  • Manual review

  • Foundry

Recommendations

The reason for reverting should be stored in memory in the same location that revert uses to locate the data it's supposed to revert with.

revert returns four bytes (0x04) starting from memory location 0x1c. A four-byte error, when stored in memory, will be padded to 32 bytes. According to this, we can easily calculate the location where the error should be stored in memory:

(0x1c + 0x04) - 0x20 = 0x0, where 0x20 is the hexadecimal representation of 32.

The error should be stored at the memory location 0x00, which is known as scratch space (Layout in Memory).

Recommended changes to the MathMasters.sol::mulWad() function:

function mulWad(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, div(not(0), y))) {
- mstore(0x40, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
+ mstore(0x00, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
revert(0x1c, 0x04)
}
z := div(mul(x, y), WAD)
}
}

Recommended changes to the MathMasters.sol::mulWadUp() function:

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()`.
+ mstore(0x00, 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))
}
}

If we attempt to execute the same test that was used in the PoC, we can see that it now passes as the function call reverted with the expected error.

Updates

Lead Judging Commences

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

Wrong error storage

Support

FAQs

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