Fit Cyan Kookaburra - Integer division without precision handling will cause inaccurate calculations for users

[sherlock-admin4]

Fit Cyan Kookaburra

Low/Info

Integer division without precision handling will cause inaccurate calculations for users

Summary

In TaxCalculator.sol, integer division without precision handling will cause inaccurate calculations for users, as it truncates decimal values, leading to incorrect compounded interest computations over time.

Root Cause

On TaxCalculator.sol:

• Line 90: The division perSecondRate / 1000 performs integer division, discarding fractional components and causing cumulative inaccuracies in the compounded factor.

  compoundedFactor_ = _previousCompoundedFactor * (1e18 + (perSecondRate / 1000 * _timePeriod)) / 1e18;

Internal pre-conditions

1. perSecondRate is less than 1000 or not a multiple of 1000, resulting in truncation when divided by 1000.
2. _timePeriod is greater than zero.
3. _previousCompoundedFactor has a significant value affecting calculations.

External pre-conditions

N/A.

Attack Path

1. Contract calculates compoundedFactor_ using integer division:

   compoundedFactor_ = _previousCompoundedFactor * (1e18 + (perSecondRate / 1000 * _timePeriod)) / 1e18;

2. Loss of precision occurs:

   • Fractional parts of perSecondRate are lost due to integer division.

3. Interest calculations become inaccurate over time:

   • Users accrue less or more interest than they should, affecting fairness.

Impact

Users will experience inaccurate interest calculations, leading to financial losses, as integer division without precision handling causes loss of fractional values in interest computations.

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract InterestCalculationTest {
    function testInterestCalculation() public pure returns (uint256 incorrectFactor, uint256 correctFactor) {
        uint256 perSecondRate = 1500; // Represents 1.5 per second
        uint256 _timePeriod = 3600;   // 1 hour in seconds
        uint256 _previousCompoundedFactor = 1e18;

        // Incorrect calculation due to integer division
        incorrectFactor = _previousCompoundedFactor * (1e18 + (perSecondRate / 1000 * _timePeriod)) / 1e18;
        // perSecondRate / 1000 == 1 (integer division), loses the 0.5

        // Correct calculation preserving precision
        correctFactor = _previousCompoundedFactor * (1e18 + (perSecondRate * _timePeriod / 1000)) / 1e18;
        // perSecondRate * _timePeriod / 1000 preserves decimal values

        // Returns both factors for comparison
        return (incorrectFactor, correctFactor);
    }
}

Explanation:

• Incorrect Calculation:

  • perSecondRate / 1000 performs integer division.
  • If perSecondRate = 1500, then perSecondRate / 1000 = 1 (losing the 0.5).
  • This results in inaccurate compounded factors over time.

• Correct Calculation:

  • Multiplying before dividing preserves the decimal:
  • perSecondRate * _timePeriod / 1000 calculates as (1500 * 3600) / 1000 = 5400000 / 1000 = 5400.
  • This method retains precision, leading to accurate interest calculations.

Mitigation

• Reorder operations to preserve precision:

  • Perform multiplication before division to maintain fractional values:

  compoundedFactor_ = _previousCompoundedFactor * (1e18 + (perSecondRate * _timePeriod / 1000)) / 1e18;

• Use Fixed-Point arithmetic:

  • Implement libraries or functions that handle decimals accurately, such as using the FixedPointMathLib:

  import {FixedPointMathLib} from "@solady/utils/FixedPointMathLib.sol";
  
  // Use mulDiv function to handle multiplication and division with precision
  compoundedFactor_ = _previousCompoundedFactor.mulDiv(
      1e18 + perSecondRate * _timePeriod,
      1e18 * 1000
  );

• Avoid integer division when precision is critical:

  • Be cautious with divisions that can truncate important decimal values.
  • Always ensure that operations preserve as much precision as necessary for accurate calculations.