15,000 USDC
View results
Submission Details
Severity: medium

DOS Vulnerability Due to Unbounded Loop Length

Summary

Vulnerability Details

Loops that do not have a fixed number of iterations, for example, loops that depend on storage values, have to be used carefully: Due to the block gas limit, transactions can only consume a certain amount of gas. Either explicitly or just due to normal operation, the number of iterations in a loop can grow beyond the block gas limit which can cause the complete contract to be stalled at a certain point.

Impact

Imagine this scenario.

  1. Contract is deployed with 2^256-1 token addresses and price feeds.

  2. Alice then deposits 10 WETH to the contract.

  3. She then decides to redeem the collateral.

  4. redeemCollateral() calls getAccountCollateralValue(address user) which loops through the array of token addresses.

  5. The gas consumed is bigger than the block gas limit causing the transaction to revert.

  6. All funds are not stuck in the contract.

function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
// loop through each collateral token, get the amount they have deposited, and map it to
// the price, to get the USD value
for (uint256 i = 0; i < s_collateralTokens.length; i++) {
//This line would cause a DOS and revert the transaction
address token = s_collateralTokens[i];
uint256 amount = s_collateralDeposited[user][token];
totalCollateralValueInUsd += getUsdValue(token, amount);
}
return totalCollateralValueInUsd;
}

Tools Used

Manual review.

Recommendations

Consider adding an upper bound for the amount of tokens and price feeds that can be added to the contract.

Support

FAQs

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