First Flight #8: Math Master

Summary

The comments showcase that the author of the library intended the MathMasters__MulWadFailed() custom error to be thrown on lines 40 and 53.

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

However, the value that is loaded into memory is '0xbac65e5b' and that is the incorrect error selector for MathMasters__MulWadFailed(). Instead, the error selector that should be used is

bytes32 hash_of_error = keccak256("MathMasters__MulWadFailed()");
bytes4 error_selector = bytes4(hash_of_error);

Vulnerability Details

The wrong error selector is loaded into memory for the revert. Also note that the free memory pointer is over written as well. Instead, use the scratch space slot. Also note that the place in memory that the revert returns is not the start of a specific slot and is not where '0xbac65e5b' is stored.

function mulWad(uint256 x, uint256 y) internal pure returns (uint256 z) {
    // @solidity memory-safe-assembly
    bytes32 hash_of_error = keccak256("MathMasters__MulWadFailed()");
    bytes4 error_selector = bytes4(hash_of_error);

    assembly {
        // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`.
        if mul(y, gt(x, div(not(0), y))) {
            mstore(0x00, error_selector) // `MathMasters__MulWadFailed()`.
            revert(0x00, 0x4)
        }
        z := div(mul(x, y), WAD)
    }
}

Impact

This will not cause any changes in logic, but could be an inconvenience to users.

Tools Used

Foundry

Proof Of Concept

function  testFailMulWadRevert() public {
    vm.expectRevert(MathMasters.MathMasters__MulWadFailed.selector);
    MathMasters.mulWad(UINT256_MAX, UINT256_MAX);

}

Note that this test is expected to fail because of the testFail naming convention.

Recommendations

As noted, load the correct error selector into the scratch space memory slot.