Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Potential integer overflow risk

biakia

Summary

The getUserDeposits function in the PerpetualVault contract uses a uint8 type for the loop variable i. This poses a potential integer overflow risk if the number of deposits (length) exceeds 255, as uint8 can only store values from 0 to 255. If length is greater than 255, the loop variable i will overflow, leading to unexpected behavior such as infinite loops or incorrect results.

Vulnerability Details

Here is the relevant code snippet:

/**
* @notice
* get all deposit ids of a user
* @param user address of a user
*/
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;
}
}
}

  • uint8 Range: The uint8 type can only store values from 0 to 255. If length exceeds 255, the loop variable i will overflow when it reaches 256, resetting to 0.

  • Consequences:

    • Infinite Loop: If length is greater than 255, the loop will never terminate because i will repeatedly overflow and reset to 0.

    • Incorrect Results: If the loop does not terminate, the function may return incomplete or incorrect data, or it may run out of gas and revert.

Example Scenario

  1. A user has 300 deposits in the userDeposits set.

  2. The length variable is set to 300.

  3. The loop starts with i = 0 and increments until i = 255.

  4. When i reaches 256, it overflows and resets to 0.

  5. The loop continues indefinitely, causing the transaction to run out of gas or revert.

Impact

The impact is Medium because the getUserDeposits function will always revert if a user has more than 256 deposits. The likelihood is Medium, so the severity is Medium.

Tools Used

Manual Review

Recommendations

Consider following fix:

/**
* @notice
* get all deposit ids of a user
* @param user address of a user
*/
function getUserDeposits(address user) external view returns (uint256[] memory depositIds) {
uint256 length = EnumerableSet.length(userDeposits[user]);
depositIds = new uint256[](length);
for (uint256 i = 0; i < length; ) {
depositIds[i] = EnumerableSet.at(userDeposits[user], i);
unchecked {
i = i + 1;
}
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.