Core Contracts

Finding Description and Impact

The function calculateTimeWeightedAverage() multiplies timeWeightedValue by weight without overflow protection. Since weight is scaled by 1e18, this operation can easily overflow, especially if timeWeightedValue is large. This could lead to incorrect calculations of the time-weighted average, potentially causing financial losses or system failures in contracts relying on accurate averages.

Root Cause:

• The multiplication timeWeightedValue * periods[i].weight is performed in an unchecked block.

• Since weight is scaled by 1e18, the product can exceed the maximum value of a uint256 if timeWeightedValue is sufficiently large.

• This overflow is not caught, leading to incorrect results.

Impact:

• Incorrect time-weighted average calculations due to overflow.

• Financial losses or system failures if the contract relies on accurate time-weighted averages.

Proof of Concept

1. Scenario:

   • Suppose timeWeightedValue = 2^200 and weight = 1e18.

   • The multiplication timeWeightedValue * weight will overflow because 2^200 * 1e18 exceeds the maximum value of a uint256 (2^256 - 1).

   • This overflow will result in an incorrect totalWeightedSum, leading to an incorrect time-weighted average.

2. Proof:

   function calculateTimeWeightedAverage(

   PeriodParams[] memory periods,

   uint256 timestamp

   ) public pure returns (uint256 weightedAverage) {

   uint256 totalWeightedSum;

   uint256 totalDuration;

   for (uint256 i = 0; i < periods.length; i++) {

   if (timestamp <= periods[i].startTime) continue;

   uint256 endTime = timestamp > periods[i].endTime ? periods[i].endTime : timestamp;

   uint256 duration = endTime - periods[i].startTime;

   unchecked {

   uint256 timeWeightedValue = periods[i].value * duration;

   if (timeWeightedValue / duration != periods[i].value) revert ValueOverflow();

   totalWeightedSum += timeWeightedValue * periods[i].weight; // Potential overflow here

   totalDuration += duration;

   }

   }

   return totalDuration == 0 ? 0 : totalWeightedSum / (totalDuration * 1e18);

   }

Recommended Mitigation Steps

To mitigate this issue, add an overflow check for the multiplication timeWeightedValue * periods[i].weight:

Explanation:

• The check if (weightedValue / periods[i].weight != timeWeightedValue) ensures that the multiplication timeWeightedValue * periods[i].weight does not overflow.

• If an overflow is detected, the function reverts with a ValueOverflow error.

Alternative: Use SafeMath Library

If the project uses OpenZeppelin's SafeMath library, you can simplify the overflow checks:

Explanation:

• The SafeMath library automatically checks for overflow during multiplication, reverting if an overflow occurs.

• This approach simplifies the code and ensures robust overflow protection.

However calculateTimeWeightedAverageis not actually called anywhere, so it could also just be deleted.