DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Denial of Service (DoS) in the PerpetualVault

Summary

The DoS in getUserDeposits arises from unbounded loops, which can be exploited by creating a large number of deposits. Functions like getUserDeposits iterate over EnumerableSet, which could run out of gas if the set grows too large. the GMX callback functions (afterOrderExecution, afterOrderCancellation) fails due to gas limits,

Vulnerability Details

The getUserDeposits function iterates over an EnumerableSet to retrieve all deposit IDs for a user. If a user has a large number of deposits, this function could run out of gas, causing the transaction to fail.

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;
}
}
}
  • An attacker repeatedly calls the deposit function to create a large number of deposits (e.g., 10,000 deposits).

  • Each deposit is small (e.g., 1 wei) to minimize cost.

Trigger DoS:

  • A legitimate user or external contract calls getUserDeposits to retrieve the deposit IDs for the attacker's address.

  • The function iterates over the large EnumerableSet, consuming excessive gas and eventually running out of gas.

Below is a simplified PoC to demonstrate the DoS vulnerability

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
contract PerpetualVault {
using EnumerableSet for EnumerableSet.UintSet;
mapping(address => EnumerableSet.UintSet) private userDeposits;
function deposit(uint256 amount) external {
// Simulate deposit logic
uint256 depositId = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender)));
userDeposits[msg.sender].add(depositId);
}
function getUserDeposits(address user) external view returns (uint256[] memory depositIds) {
uint256 length = userDeposits[user].length();
depositIds = new uint256[](length);
for (uint256 i = 0; i < length; i++) {
depositIds[i] = userDeposits[user].at(i);
}
}
}
contract Attack {
PerpetualVault private vault;
constructor(address _vault) {
vault = PerpetualVault(_vault);
}
function attack() external {
// Create 10,000 deposits to trigger DoS
for (uint256 i = 0; i < 10000; i++) {
vault.deposit(1); // Deposit 1 wei
}
}
}

Steps to Reproduce

  1. Deploy the PerpetualVault contract.

  2. Deploy the Attack contract, passing the address of the PerpetualVault contract.

  3. Call the attack function on the Attack contract to create 10,000 deposits.

  4. Call getUserDeposits on the PerpetualVault contract with the attacker's address.

  5. Observe that the transaction runs out of gas and fails.

Impact

  • The getUserDeposits function fails, preventing the user or contract from retrieving deposit IDs.

  • If this function is used in critical operations (e.g., withdrawals), the entire system could be disrupted.

Tools Used

Manual Code Reviiew

Recommendations

  • Implement pagination to limit the number of items returned in a single call.

Example:

function getUserDeposits(address user, uint256 start, uint256 limit) external view returns (uint256[] memory depositIds) {
uint256 length = userDeposits[user].length();
if (start >= length) return new uint256[](0);
uint256 end = start + limit;
if (end > length) end = length;
depositIds = new uint256[](end - start);
for (uint256 i = start; i < end; i++) {
depositIds[i - start] = userDeposits[user].at(i);
}
}
  • Restrict the number of deposits a single user can create.

Example:

uint256 public constant MAX_DEPOSITS_PER_USER = 100;
function deposit(uint256 amount) external {
require(userDeposits[msg.sender].length() < MAX_DEPOSITS_PER_USER, "Max deposits reached");
uint256 depositId = uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender)));
userDeposits[msg.sender].add(depositId);
}
  • Optimize gas usage in loops by minimizing storage reads and writes.

function getUserDeposits(address user) external view returns (uint256[] memory depositIds) {
uint256 length = userDeposits[user].length();
depositIds = new uint256[](length);
EnumerableSet.UintSet storage deposits = userDeposits[user];
for (uint256 i = 0; i < length; i++) {
depositIds[i] = deposits.at(i);
}
}
  • Use an off-chain service to index deposit IDs and provide them via an API.

Updates

Lead Judging Commences

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

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