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

DoS

0xakira

Vulnerability details

The function getUserDeposits is intended for the user to see his id of each deposit. This function uses a loop to add each id to the array, but this function will not be available to the user if the number of his deposits exceeds 255

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;
}
}
}

Impact

If the number of user deposits exceeds 255 (uint8.max ==255), the user will not be able to call
this function because it will be in an infinite loop.

Let's consider such a scenario:

  1. User has 256 deposits and wants to know their id.

  2. He calls this function.

  3. The variable i is of type uint8, the loop will exist as long as i <256

  4. the loop becomes infinite as the variable i is incremented in the unchecked block, thus reaching 255 , the variable i is updated to 0 and all over again.

Since this probability is very small, so the severity of the low

Proof of Concept

Paste the following code into PerpetualVault.t.sol

function test_getDepositsDoS() public{
IERC20 token = IERC20(PerpetualVault(vault).collateralToken());
address user1 = makeAddr("user1");
deal(address(token),user1, 10 ether);
vm.startPrank(user1);
token.approve(vault, type(uint256).max);
for (uint256 i = 0; i < 256; i++) {
PerpetualVault(vault).deposit(PerpetualVault(vault).minDepositAmount());
}
PerpetualVault(vault).getUserDeposits(user1);
}

Call this test

forge test --mt test_getDepositsDoS --via-ir --rpc-url arbitrum -vvvv
​
...
​
← [OutOfGas] EvmError: OutOfGas
│ └─ ← [Revert] EvmError: Revert
└─ ← [Revert] EvmError: Revert
​
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 4.17s (4.16s CPU time)
​
Ran 1 test suite in 5.55s (4.17s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/PerpetualVault.t.sol:PerpetualVaultTest
[FAIL: EvmError: Revert] test_getDepositsDoS() (gas: 1041896328)

Recommended Mitigation Steps

Change the type of the variable i from uint8 to at least uint16

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; ) {
+ for (uint16 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.