rcadob - Unchecked arithmetic operations will lead to incorrect calculations for users

[sherlock-admin4]

rcadob

High

Unchecked arithmetic operations will lead to incorrect calculations for users

Summary

In multiple files (TaxCalculator.sol, LinearRangeCurve.sol, Listings.sol, ProtectedListings.sol, UniswapImplementation.sol), unchecked arithmetic operations will lead to incorrect calculations for users as underflows and overflows may occur without proper validation.

Root Cause

Arithmetic operations are performed without ensuring values remain within data type bounds:

• Underflow Errors:

  • TaxCalculator.sol:

  • Line 69: Subtraction _utilizationRate - UTILIZATION_KINK may underflow if _utilizationRate is less than UTILIZATION_KINK.

    interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;

  • LinearRangeCurve.sol:

  • Line 60: Subtraction end - start may underflow if end is less than start.

    inputValue = numItems * (spotPrice * (end - block.timestamp) / (end - start));

  • Listings.sol:

  • Line 933: Subtraction _listing.duration - (block.timestamp - _listing.created) may underflow if block.timestamp - _listing.created exceeds _listing.duration.

    refund_ = (_listing.duration - (block.timestamp - _listing.created)) * taxPaid / _listing.duration;

• Overflow Errors:

  • TaxCalculator.sol:

  • Line 90: Multiplication of large values without checks may overflow.

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

  • ProtectedListings.sol:

  • Line 273: Multiplication of large numbers may overflow.

    utilizationRate_ = (listingsOfType_ * 1e36 * 10 ** collectionToken.denomination()) / totalSupply;

  • UniswapImplementation.sol:

  • Line 607: Multiplying swapAmount by ammFee may overflow.

    uint feeAmount = uint128(swapAmount) * ammFee / 100_000;

Internal pre-conditions

1. Underflow Conditions:

   • TaxCalculator.sol Line 69:
     • _utilizationRate is less than UTILIZATION_KINK.
   • LinearRangeCurve.sol Line 60:
     • end is less than start.
   • Listings.sol Line 933:
     • block.timestamp exceeds _listing.created + _listing.duration.

2. Overflow Conditions:

   • TaxCalculator.sol Line 90:
     • _previousCompoundedFactor, perSecondRate, or _timePeriod are large enough to cause overflow.
   • ProtectedListings.sol Line 273:
     • listingsOfType_ and collectionToken.denomination() are large.
   • UniswapImplementation.sol Line 607:
     • swapAmount and ammFee are large values close to their maximum limits.

External pre-conditions

N/A.

Attack Path

1. Users invoke functions with vulnerable arithmetic operations:

   • For example, functions that calculate interest rates, utilization rates, or fees.

2. Underflow or overflow occurs during calculations:

   • Arithmetic operations exceed the data type limits.

3. Incorrect values are computed, or transactions revert:

   • Users receive wrong amounts or experience transaction failures.

Impact

The users suffer incorrect calculations, which can lead to financial losses or contract instability due to underflow and overflow errors in arithmetic operations.

PoC

Underflow in TaxCalculator.sol Line 69:

uint256 UTILIZATION_KINK = 0.8 ether;
uint256 _utilizationRate = 0.5 ether; // Less than UTILIZATION_KINK

// Underflow occurs here
uint256 interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;
// _utilizationRate - UTILIZATION_KINK underflows

Overflow in UniswapImplementation.sol Line 607:

uint128 swapAmount = type(uint128).max; // Maximum uint128 value
uint256 ammFee = 100000; // Large fee

// Potential overflow during multiplication
uint256 feeAmount = uint128(swapAmount) * ammFee / 100_000;
// Multiplication overflows, resulting in incorrect feeAmount

Overflow in UniswapImplementation.sol Line 607:

uint128 swapAmount = type(uint128).max; // Maximum uint128 value
uint256 ammFee = 100000; // Large fee

// Potential overflow during multiplication
uint256 feeAmount = uint128(swapAmount) * ammFee / 100_000;
// Multiplication overflows, resulting in incorrect feeAmount

Mitigation

• Use Safe Arithmetic Operations:

  • Utilize Solidity 0.8's built-in overflow and underflow checks.

• Add Validation Checks:

  • For Underflow:

  • TaxCalculator.sol Line 69:

    require(_utilizationRate >= UTILIZATION_KINK, "Utilization rate must be >= UTILIZATION_KINK");

  • LinearRangeCurve.sol Line 60:

    require(end > start, "End time must be greater than start time");

  • Listings.sol Line 933:

    uint256 timeElapsed = block.timestamp - _listing.created;
    require(timeElapsed <= _listing.duration, "Listing has already ended");

  • For Overflow:

  • TaxCalculator.sol Line 90:

    // Ensure values are within safe limits
    require(perSecondRate * _timePeriod / 1000 <= MAX_ALLOWED_VALUE, "Value too large");

  • ProtectedListings.sol Line 273:

    // Use safe multiplication functions or check for potential overflows
    require(listingsOfType_ <= MAX_LISTINGS, "Too many listings");

  • UniswapImplementation.sol Line 607:

    // Ensure swapAmount and ammFee are within safe ranges
    require(uint256(swapAmount) * ammFee / 100_000 <= MAX_FEE_AMOUNT, "Fee amount overflow");

• Implement Safe Math Libraries:

  • Use libraries like OpenZeppelin's SafeMath for versions of Solidity before 0.8.

• Limit Input Values:

  • Set maximum allowable values for inputs that could cause overflows.