Root + Impact
Description
In Solidity, integer division truncates any fractional part, meaning 5 / 2 results in 2. To maintain precision in calculations involving both multiplication and division, it is generally best practice to perform multiplication before division.
The issue is that in several instances within Math.mulDiv and Math.invMod, a division operation is performed before a subsequent multiplication. This order of operations can lead to a loss of precision due to intermediate truncation, potentially causing the final result to be less accurate than intended.
SLITHER OUTPUT:
## divide-before-multiply
Impact: Medium
Confidence: Medium
- [ ] ID-1
[Math.mulDiv(uint256,uint256,uint256)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275) performs a multiplication on the result of a division:
- [denominator = denominator / twos](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L242)
- [inverse *= 2 - denominator * inverse](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L265)
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275
- [ ] ID-2
[Math.mulDiv(uint256,uint256,uint256)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275) performs a multiplication on the result of a division: - [denominator = denominator / twos](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L242) - [inverse = (3 * denominator) ^ 2](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L257)
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275
- [ ] ID-3
[Math.mulDiv(uint256,uint256,uint256)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275) performs a multiplication on the result of a division:
- [low = low / twos](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L245)
- [result = low * inverse](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L272)
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275
- [ ] ID-4
[Math.invMod(uint256,uint256)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L315-L361) performs a multiplication on the result of a division:
- [quotient = gcd / remainder](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L337)
- [(gcd,remainder) = (remainder,gcd - remainder * quotient)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L339-L346)
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L315-L361
- [ ] ID-5 [Math.mulDiv(uint256,uint256,uint256)](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275) performs a multiplication on the result of a division:
- [denominator = denominator / twos](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L242)
- [inverse *= 2 - denominator * inverse](lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L263)
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L204-L275
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L242
denominator = denominator / twos;
inverse \*= 2 - denominator \* inverse;
<br />
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L257
denominator = denominator / twos;
inverse = (3 \* denominator) ^ 2;
<br />
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L245
low = low / twos;
result = low \* inverse;
<br />
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L337
quotient = gcd / remainder;
(gcd,remainder) = (remainder,gcd - remainder \* quotient);
Risk
Likelihood:
This will occur whenever the affected mathematical functions are executed with inputs that would result in non-exact integer divisions, leading to truncation.
This will occur consistently in scenarios where the intermediate division significantly reduces the value before subsequent multiplication, particularly for small numbers.
Impact:
Accumulated rounding errors in calculations, leading to discrepancies in sensitive values like token amounts, prices, or rewards.
Slightly incorrect final results, which might be acceptable for some applications but critical for others where high precision is required.
Proof of Concept
uint256 initialValue = 100;
uint256 factor = 3;
uint256 divisor = 2;
uint256 result1 = (initialValue / divisor) * factor;
uint256 result2 = (initialValue * factor) / divisor;
Recommended Mitigation
- remove this code
+ add this code
--- a/lib/openzeppelin-contracts/contracts/utils/math/Math.sol
+++ b/lib/openzeppelin-contracts/contracts/utils/math/Math.sol
@@ -241,12 +241,12 @@
}
- denominator = denominator / twos;
- low = low / twos;
+
+
+
+
+
twos = twos * 2;
}
- inverse *= 2 - denominator * inverse;
- inverse *= 2 - denominator * inverse;
- result = low * inverse;
}
}