Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

Incorrect custom error implementation in mulWadUp()

Summary

The mulWadUp() function has a condition where it reverts with a custom error, but the returned signature is incorrect.

Vulnerability Details

According to the documentation, the custom error should be MathMasters__MulWadFailed() with signature 0xa56044f7, but the returned signature is 0xbac65e5b, which corresponds to MulWadFailed()

In addition, the mstore opcode is overriding the free memory pointer (0x40), which and lead to serious errors. The revert is finally returning 0x00000000 because the first 32-byte slot is not being written.

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

Dapps or contracts that interact with the contract implementing this library will get incorrect information about the error, and may malfunction.

Tools Used

Foundry, Manual review

Proof of Concept

This is a test that triggers the revert and expects the custom error signature 0xa56044f7.

function testMulWadUpRevert() public {
uint256 x = type(uint256).max / 2;
uint256 y = 100;
vm.expectRevert(0x00000000);
uint256 r = MathMasters.mulWadUp(x, y);
assertEq(r, (x * y - 1) / 1e18 + 1);
}

Test passes, confirming that the returned signature is empty.

forge test --mt testMulWadRevert
...
[PASS] testMulWadUpRevert() (gas: 3197)

Recommended Mitigation

Correct the custom error and memory location.

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, div(not(0), y))) {
- mstore(0x40, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
+ mstore(0x00, 0xa56044f7) // `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:

Wrong error selector

Wrong error storage

Support

FAQs

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