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

Customer error is inconsistent to revert data in assembly code

Summary

Custom error is introduced since Solidity 0.8.4, allowing developer to showcase different error events. Consider the following error event:

error Unauthorized();

And the code section below revert the transaction with Unauthorized error.

revert Unauthorized();

It is equivalent to the Yul code as follow:

let freeMemPtr := mload(0x40)
mstore(freeMemPtr, Unauthorized.selector)
revert(freeMemPtr, 0x04)

The freeMemPtr first load the free memory pointer into the variable, and loading the signature of custom error Unauthorized, finally revert the transaction with the signature.

However, the value in mstore opcode is mismatched with the error signature.

Vulnerability Details

In MathMasters::mulWadUp() and MathMasters::mulWad() functions, there are custom error specified, which is MathMasters__MulWadFailed(). However, the value in revert opcode is inconsistent to the error signature.

add the following test in test/MathMasters.t.sol,

function testMulWadErroreEvent() public pure {
assertEq(MathMasters__MulWadFailed.selector, 0xbac65e5b);
}

The test will fail, indicating the revert data is incorrect.

Impact

The parameter passed in revert opcode is inconsistent to the comment, and it will confuse the developer about the error message.

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()`.
revert(0x1c, 0x04) // @TODO check 0x1c data
}
z := div(mul(x, y), WAD)
}

Tools Used

Foundry

Recommendations

correct the error signature, updating 0xbac65e5b to 0xe243bb9b0, which is the signature of MathMasters__MulWadFailed error.

mulWad(uint256 x, uint256 y)

assembly {
// Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
if mul(y, gt(x, div(not(0), y))) {
- mstore(0x40, 0xbac65e5b) // Incorrect `MathMasters__MulWadFailed()` signature
+ mstore(0x40, 0xe243bb9b) // Correct `MathMasters__MulWadFailed()` signature
revert(0x1c, 0x04)
}
z := div(mul(x, y), WAD)
}

mulWadUp(uint256 x, uint256 y)

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) // Correct `MathMasters__MulWadFailed()` signature
+ mstore(0x40, 0xe243bb9b) // Incorrect `MathMasters__MulWadFailed()` signature
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
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong error selector

Support

FAQs

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