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

Not Memory-safe-assembly in `MathMasters::mulWadUp()` and `MathMasters::mulWad()` function leads to blank revert.

ereynier

Summary

The assembly blocks are marked as @solidity memory-safe-assembly but they are not.

Vulnerability Details

You use the free memory pointer address 0x40 to store the error selector. This address is used by the EVM to store the free memory pointer. You should use 0x00 instead as the selector can fit in.

Impact

The functions will return a blank error instead of the custom error. An external script or user could watch for this error and never find it.

PoC (Proof of Code)

Add this lines at the end of MathMastersTest.

function testMulWadUpRevert() public {
uint256 x = 4;
uint256 y = type(uint256).max / 2;
bytes4 selector = MathMasters.MathMasters__MulWadFailed.selector;
vm.expectRevert(selector);
MathMasters.mulWad(x, y);
}

Tools Used

  • Foundry

  • https://docs.soliditylang.org/en/v0.8.17/assembly.html#memory-safety

  • https://docs.soliditylang.org/en/v0.8.17/internals/layout_in_memory.html#layout-in-memory

Recommendations

Use the "scratch space" to store the selector. You can use mstore(0, selector) instead of mstore(0x40, selector), and revert(0, 4) instead of revert(0x1c, 0x04).

function mulWad(uint256 x, uint256 y) internal pure returns (uint256 z) {
// @solidity memory-safe-assembly
+ bytes4 selector = MathMasters__MulWadFailed.selector; // other report | use the good selector
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)
+ mstore(0, selector) // `MathMasters__MulWadFailed()`.
+ revert(0, 4)
}
z := div(mul(x, y), WAD)
}
}
function mulWadUp(uint256 x, uint256 y) internal pure returns (uint256 z) {
// @solidity memory-safe-assembly
+ bytes4 selector = MathMasters__MulWadFailed.selector; // other report | use the good selector
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)
+ mstore(0, selector) // `MathMasters__MulWadFailed()`.
+ revert(0, 4)
}
.
.
}
}
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 storage

Support

FAQs

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