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

mulWadUp fails overflow check

ilovemiagoth

Summary

Overflow is not caught in inline assembly and multiplication yields gibberish value.

Vulnerability Details

In line 52, an x * y overflow check is done with:

// Equivalent to require(y == 0 || x <= type(uint256).max / y).
if mul(y, gt(x, or(div(not(0), y), x)))

However, it is not equivalent to the require in the comment and we can check with Forge:

function testMulWadUpTwoLargeUints() public{
uint256 x = type(uint256).max;
uint256 result = MathMasters.mulWadUp(x, x);
}

The function above runs normally without a revert and yields 0 as a result.

Impact

This is a critical bug because a protocol using this library might be counting on the fact that overflows are caught correctly, thus causing logic issues whenever this function is used.

Tools Used

Forge.

Recommendations

I recommend replacing line 52 with:

if mul(y, gt(x, div(not(0), y))) { // notice the absence of the or() instruction.

The overflow check is correctly implemented in mulWad, at line 39:
if mul(y, gt(x, div(not(0), y))) {

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`mulWadUp` has a bad overflow check

Support

FAQs

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