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

Incorrect memory slot for loading the error selector

Ritos

Summary

The mulWad(uint256 x, uint256 y) and mulWadUp(uint256 x, uint256 y) functions use an invalid pointer to error selector in revert.

Vulnerability Details

In functions mulWad(uint256 x, uint256 y) and mulWadUp(uint256 x, uint256 y) we store error selector start from 0x40 (which is also incorrect, as described in the previous finding )

mstore(0x40, 0xbac65e5b)

This means that we write 32 bytes: 28 left bytes of which are filled with zeros, because the error selector occupies only 4 right bytes.
When we want to read error selector in revert function
https://github.com/Cyfrin/2024-01-math-master/blob/84c149baf09c1558d7ba3493c7c4e68d83e7b3aa/src/MathMasters.sol#L41

revert(0x1c, 0x04)

we need to add 28 bytes for the pointer that we used in mstore.
Therefore, we see that pointer 0x1c does not conform to this logic and does not point to the beginning of the error selector location.

Impact

Uninformative output when an error occurs.

Tools Used

Manual review.

Recommendations

These recommendations take into account the recommendations from the previous finding (about rewriting free memory pointer).

/// @dev Equivalent to `(x * y) / WAD` rounded down.
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()`.
- revert(0x1c, 0x04)
+ mstore(0x20, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
+ revert(0x3c, 0x04)
}
z := div(mul(x, y), WAD)
}
}
​
/// @dev Equivalent to `(x * y) / WAD` rounded up.
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)
+ mstore(0x20, 0xbac65e5b) // `MathMasters__MulWadFailed()`.
+ revert(0x3c, 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 storage

Ritos Submitter
over 1 year ago
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.