Beatland Festival

First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

divide-before-multiply

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
// Root cause in the codebase with @> marks to highlight the relevant section
// ID-1, ID-5 from Math.mulDiv
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L242
denominator = denominator / twos; // @> Division before multiplication on next line
// ...
inverse \*= 2 - denominator \* inverse;
<br />
// ID-2 from Math.mulDiv
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L257
denominator = denominator / twos; // @> Division before multiplication on next line
// ...
inverse = (3 \* denominator) ^ 2; // This line also contains the \`incorrect-exp\` bug
<br />
// ID-3 from Math.mulDiv
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L245
low = low / twos; // @> Division before multiplication on next line
// ...
result = low \* inverse;
<br />
// ID-4 from Math.invMod
lib/openzeppelin-contracts/contracts/utils/math/Math.sol#L337
quotient = gcd / remainder; // @> Division before multiplication on next line
// ...
(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

// Example demonstrating precision loss due to order of operations
uint256 initialValue = 100;
uint256 factor = 3;
uint256 divisor = 2;
// Incorrect order (divide then multiply)
uint256 result1 = (initialValue / divisor) * factor; // (100 / 2) * 3 = 50 * 3 = 150
// Correct order (multiply then divide)
uint256 result2 = (initialValue * factor) / divisor; // (100 * 3) / 2 = 300 / 2 = 150
// In this specific example, the results are the same due to exact division.
// However, consider if initialValue = 10, divisor = 3, factor = 2
// Incorrect: (10 / 3) * 2 = 3 * 2 = 6 (Loss of 0.33 * 2 = 0.66 precision)
// Correct: (10 * 2) / 3 = 20 / 3 = 6 (This is still truncated, but for (X * Y) / Z, it's generally more precise than (X / Z) * Y when dealing with integers)
// A common pattern where this is important: Calculating a percentage `amount * percentage / 100` instead of `amount / 100 * percentage`.

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 @@
}
// Iterate until x is in the 1.0 to 2.0 range.
- denominator = denominator / twos; // @> Refactor: if possible, reorder operations
- low = low / twos; // @> Refactor: if possible, reorder operations
+ // Example change (conceptual, specific refactor depends on Math lib logic):
+ // denominator_temp = denominator;
+ // denominator = denominator * precision_factor / twos;
+ // low_temp = low;
+ // low = low * precision_factor / twos;
twos = twos * 2;
}
- inverse *= 2 - denominator * inverse;
- inverse *= 2 - denominator * inverse;
- result = low * inverse;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
25 days ago
inallhonesty Lead Judge 24 days ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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