Unchecked Multiplication Leading to Overflow in Fixed-Point Arithmetic Operations

Summary

The MathLib library in the provided Solidity code performs multiplication before division in its mulDivDown and mulDivUp functions. This approach can lead to overflow when the product of the operands exceeds the maximum value for a uint256. This vulnerability can result in incorrect calculations, potentially leading to financial loss or incorrect contract behavior.

Vulnerability Detail

The mulDivDown and mulDivUp functions do not include overflow checking during the multiplication step.

  function mulDivDown(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) {
@=> return (x * y) / d;
  }

  function mulDivUp(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) {
@=> return (x * y + (d - 1)) / d;
  }

Scenario: Consider the following scenario where x and y are large enough to cause overflow:

uint256 x = 2**128;
uint256 y = 2**128;
uint256 d = 1;

// This will overflow as (2**128 * 2**128) exceeds uint256 max value.
uint256 result = MathLib.mulDivDown(x, y, d);

The multiplication should yield a value of 2**256, which is impossible to represent in a uint256 variable, leading to overflow and incorrect results.

Impact

Can lead to incorrect financial calculations.
Contracts that rely on these calculations for important financial operations can distribute funds incorrectly.

Code Snippet

Tool used

Manual Review

Recommendation

Apply a safe multiplication approach.

+library SafeMathLib {
+    function safeMul(uint256 a, uint256 b) internal pure returns (uint256) {
+        if (a == 0 || b == 0) return 0;
+        uint256 c = a * b;
+        require(c / a == b, "Multiplication overflow");
+        return c;
    }
}

library MathLib {
+    using SafeMathLib for uint256;

    function mulDivDown(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) {
+        uint256 product = x.safeMul(y);
+        return product / d;
-        return (x * y) / d;
    }

    function mulDivUp(uint256 x, uint256 y, uint256 d) internal pure returns (uint256) {
+        uint256 product = x.safeMul(y);
+        return (product + (d - 1)) / d;
-        return (x * y + (d - 1)) / d;
    }
}

sherlock-audit / 2024-06-new-scope-judging

JuggerNaut63 - Unchecked Multiplication Leading to Overflow in Fixed-Point Arithmetic Operations #87