Liquidity Management

Within the PerpetualVault (or related) contract, the function getUserDeposits(address user) retrieves an array of deposit IDs for a specified user. The audit revealed that using a uint8 loop counter to iterate over potentially large sets of deposit IDs introduces the risk of an overflow once 256 deposits are reached.

Vulnerability Details

Issue: Loop Counter Overflow in getUserDeposits

• Location:

  function getUserDeposits(address user) external view returns (uint256[] memory depositIds) {

  uint256 length = EnumerableSet.length(userDeposits[user]);

  depositIds = new uint256[](length);

  for (uint8 i = 0; i < length; ) {

  depositIds[i] = EnumerableSet.at(userDeposits[user], i);

  unchecked {

  i = i + 1;

  }

  }

  }

  ​

• Explanation:

  1. uint8 can only represent values from 0 to 255.

  2. If length > 255, the counter i would wrap around to 0 again, causing erroneous or indefinite looping.

  3. This can result in missing entries in the returned depositIds or a failure to exit the loop properly (if safe math isn’t enforced).

• Example Trigger:
  A user with 300 different deposit IDs in userDeposits[user] would cause the for-loop to overflow once i increments past 255.

Impact

• Data Integrity:

  • The user calling getUserDeposits would not receive a correct (or complete) set of deposit IDs.

  • Depending on logic built on these results, further operations (e.g., withdrawals, UI queries, or portfolio calculations) could be misleading or fail.

• Reentrancy or Lock-Up:

  • If the function accidentally enters a scenario that never meets i < length, it could lead to a runtime error or unbounded loop in extreme edge cases.

  • The immediate direct financial impact is minimal because this function is view, but the data misrepresentation can be severe.

Tools Used

• Manual Code Review: Reading through the loop logic and type usage, identifying the mismatch between uint8 i and potentially large array lengths.

Recommendations

1. Use a Larger Integer Type
   Change the loop variable from uint8 to uint256 to accommodate any length of deposits:

   for (uint256 i = 0; i < length; ) {

   ...

   unchecked {

   i++;

   }

   }

   ​

   This is the simplest fix and prevents overflow scenarios at 255 → 0.

2. Check for Array Boundaries
   Although the code currently retrieves from an EnumerableSet, always ensure you do not exceed array bounds by verifying i < length prior to accessing depositIds[i].