Dria

Summary

The getRoundPhase() function in the BuyerAgent contract incorrectly accumulates the total number of rounds by adding an unnecessary +1 in each iteration, without checking if the the result got rounded down, because of Solidity decimal handling. This miscalculation inflates the total round count, leading to incorrect determination of the current round and phase. Consequently, this can cause the contract to behave unexpectedly, potentially disrupting the protocol's operation and affecting users' interactions.

Vulnerability Details

The function aims to determine the current round, phase, and the time until the next phase based on elapsed time and market parameters. The issue arises from the way the total rounds are accumulated:

• Incorrect Accumulation Logic:

  round += innerRound + 1;

  ​

  The innerRound variable already represents the number of complete rounds within a given time interval. Adding +1 unnecessarily increments the total round count, leading to an inflated number. This addition seems to be required if we didn't have whole numbers (i.e 10) and the result has a decimal part (i.e. 10,5), which will get truncated. Since we want next/new round has round the +1 offset makes sense in order to point to the correct number of rounds. However, if the result didn't have a decimal part (i.e. was exactly 10), we add an +1 offset that results in miscalculations of rounds.

Example Illustrating the Issue

Let's consider a concrete example to demonstrate how this incorrect accumulation affects the round calculation.

Scenario Setup

• Time Intervals:

  • t0 (createdAt) = 0

  • t1 = 1000

  • t2 = 2000

  • t3 (block.timestamp) = 3000

• Market Parameters (marketParams):

  • marketParams[0]: Active from t0 to t1 with cycleTime0

  • marketParams[1]: Active from t1 to t2 with cycleTime1

  • marketParams[2]: Active from t2 onwards with cycleTime2

• Cycle Times:

  • Under marketParams[0]: cycleTime0 = 100

  • Under marketParams[1]: cycleTime1 = 200

  • Under marketParams[2]: cycleTime2 = 100

Expected Calculations

1. Rounds under marketParams[0]:

   Rounds = (t1 - t0) / cycleTime0

   = (1000 - 0) / 100

   = 10 rounds

   ​

2. Rounds under marketParams[1]:

   Rounds = (t2 - t1) / cycleTime1

   = (2000 - 1000) / 200

   = 5 rounds

   ​

3. Rounds under marketParams[2]:

   Rounds = (t3 - t2) / cycleTime2

   = (3000 - 2000) / 100

   = 10 rounds

   ​

4. Total Expected Rounds:

   Total Rounds = Rounds under params0 + Rounds under params1 + Rounds under params2

   = 10 + 5 + 10

   = 25 rounds

   ​

Calculations Using the Original Code

Using the incorrect accumulation logic from the code:

1. First Iteration:

   (uint256 round,,) = _computePhase(marketParams[0], t1 - createdAt);

   // round = 10

   ​

2. Accumulating Rounds with '+1':

   • First Accumulation:

     round += innerRound + 1; // innerRound from params[1]

     round += 5 + 1; // innerRound = 5

     round = 10 + 6 = 16

     ​

   • Second Accumulation:

     round += lastRound + 1; // lastRound from params[2]

     round += 10 + 1; // lastRound = 10

     round = 16 + 11 = 27

     ​

3. Total Rounds Calculated:

   Total Rounds = 27 rounds

   ​

Analysis

• Inflated Total Rounds:

  • Expected Total Rounds: 25

  • Calculated Total Rounds: 27

• The extra +1 in each accumulation step adds unnecessary rounds, causing the total round count to be higher than it should be.

Root Cause

The incorrect addition of +1 when accumulating rounds is the root cause of the miscalculation:

The innerRound variable already accounts for the complete rounds within the elapsed time. Adding +1 effectively counts an extra round that doesn't exist.

Impact

The issue leads to the BuyerAgent completely skipping some rounds and participate in later rounds. Furthermore, the rounds that are returned don't correlate to the correct round.

Tools Used

Manual Review

Recommended Mitigation

The +1 offset appears to be indeed required in scenarios where the decimal precision is lost in Solidity. However, when the result doesn't get rounded down and doesn't have a decimal part, the offset is not needed and leads to wrong round calculations.