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

Integer Overflow in getUserDeposits Function Causes Reverts for Large Deposit Histories

ravindu_santhush

Summary

The getUserDeposits function in the PerpetualVault contract is vulnerable to an integer overflow bug due to the use of a uint8 loop index when iterating through a user's deposit history. If a user has more than 256 deposits, the loop counter will overflow, causing incorrect execution behavior and potentially leading to infinite looping or unexpected reverts.

Vulnerability Details

The issue arises in the following code snippet inside the getUserDeposits function:

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; ) { // <-- uint8 index causes overflow if length > 255
depositIds[i] = EnumerableSet.at(userDeposits[user], i);
unchecked {
i = i + 1;
}
}
}

Proof of Concept (PoC)

The following test case demonstrates the vulnerability:

function test_GetUserDepositsOverflow() external {
address bob = makeAddr("bob");
uint256 depositAmount = 1e12; // Example deposit amount
​
// Simulate bob making 300 deposits.
for (uint256 i = 0; i < 300; i++) {
vm.startPrank(bob);
// Provide bob with sufficient collateral and approve deposit.
deal(
address(PerpetualVault(vault).collateralToken()),
bob,
depositAmount
);
IERC20(PerpetualVault(vault).collateralToken()).approve(
vault,
depositAmount
);
uint256 execFee = PerpetualVault(vault).getExecutionGasLimit(true);
// Assume deposit() works without reverting for each deposit.
PerpetualVault(vault).deposit{value: execFee * tx.gasprice}(
depositAmount
);
vm.stopPrank();
}
​
// Expect the function to revert due to uint8 overflow
vm.expectRevert();
uint256[] memory deposits = PerpetualVault(vault).getUserDeposits(bob);
}
​

Impact

Function Reverts: If a user exceeds 256 deposits, calling getUserDeposits will fail, preventing them from retrieving their deposit history.
Incorrect Data Returned: If not handled correctly, the loop could return incomplete or incorrect deposit records.
Potential Denial of Service (DoS): Users with large deposit histories might be unable to retrieve their deposits, hindering withdrawals or accounting processes.

Tools Used

Manual Code Review

Recommendations

To fix this issue, update the loop index from uint8 to uint256:

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; ) { // Change uint8 to uint256
depositIds[i] = EnumerableSet.at(userDeposits[user], i);
unchecked {
i = i + 1;
}
}
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 9 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.

Give us feedback!