Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Increasing Length Array as Loop Variable vulnerability inside `VaultControllerStrategy`

0bingo76

Summary

Increasing Length Array as Loop Variable vulnerability inside VaultControllerStrategy. This issue arises from an unbounded loop that iterates over the vaults array without any constraints on its size. As the vaults array can continuously grow without a mechanism to decrease its length, the loop's gas consumption can become unmanageable. This presents a risk of transaction failures due to gas limit exceedance, potentially leading to a denial of service (DoS) as the number of vaults increases over time.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/base/VaultControllerStrategy.sol#L496

Vulnerability Details

The vulnerability is in the VaultControllerStrategy.sol .

function getDepositChange() public view virtual returns (int) {
uint256 totalBalance = token.balanceOf(address(this));
for (uint256 i = 0; i < vaults.length; ++i) {
totalBalance += vaults[i].getTotalDeposits();
}
return int(totalBalance) - int(totalDeposits);
}
​

The loop uses vaults.length as its terminating condition. Since the vaults array is dynamic and can only grow (with no mechanism to remove elements), the length of the array will perpetually increase as more vaults are added. As transactions invoking this loop become more expensive, users may be unable to execute critical functions within the contract, effectively rendering the contract's functionality unusable. Functions relying on this loop may behave unpredictably. For instance, some users might successfully execute transactions if they forward sufficient gas, while others may consistently fail, leading to inconsistent contract behavior.

Consider a scenario where the VaultControllerStrategy manages numerous vaults. Each addition of a new vault increases the size of the vaults array. Functions that need to iterate over all vaults to perform operations (e.g., distributing rewards, auditing vault states) will have their gas costs escalate with each new vault. Over time, these operations may become so gas-intensive that they exceed the blockchain's gas limits, preventing successful execution and degrading the contract's utility.

Impact

As the number of vaults increases, functions containing the unbounded loop may consume excessive gas, leading to transaction failures. This can prevent users from performing essential operations, effectively halting the contract's functionality. Critical functions may become inaccessible, preventing users from interacting with the contract as intended. This can result in financial losses, especially if users are unable to withdraw funds or perform necessary actions in a timely manner. If core functionalities rely on iterating over all vaults, the entire contract's operations could become non-functional once the vaults array grows beyond a manageable size.

Vulnerability

The VaultManager contract stores all vault addresses in an array called vaults. Critical functions, like distributeRewards(), iterate over this array to perform operations on each vault. As new vaults are created, they are added to this array, which can grow indefinitely.

Attack Scenario

**MaliciousVault**: A malicious implementation of the `IVault` interface. Its `deposit` function performs a large number of operations to intentionally consume excessive gas, exacerbating the issue when `distributeRewards` is called.
​
**Attack Contract**: Facilitates the exploitation of the vulnerability by:
- **Adding Malicious Vaults**: Deploys multiple instances of `MaliciousVault` and adds them to the `VaultControllerStrategy`'s `vaults` array.
- **Triggering DoS**: Attempts to call the `distributeRewards` function, which, due to the inflated `vaults` array and the gas-intensive `deposit` functions of `MaliciousVaults`, will exceed the block gas limit and fail.
​
### Step-by-Step Attack Flow
​
**Deployment**:
- Deploy the `VulnerableVaultControllerStrategy` contract.
- Deploy the `Attack` contract, passing the address of the `VulnerableVaultControllerStrategy` to its constructor.
​
**Executing the Attack**:
- **Adding Malicious Vaults**:
- Call the `executeAttack` function on the `Attack` contract with a specified number of vaults (e.g., 100).
- This function loops `numberOfVaults` times, deploying new `MaliciousVault` contracts and adding their addresses to the `VulnerableVaultControllerStrategy`'s `vaults` array via the `addVault` function.
​
- **Triggering DoS**:
- After inflating the `vaults` array with malicious entries, call the `triggerDoS` function on the `Attack` contract with a chosen `rewardAmount`.
- This function attempts to execute the `distributeRewards` function on the `VulnerableVaultControllerStrategy`.
- Due to the presence of numerous `MaliciousVault` instances, each having gas-intensive `deposit` functions, the total gas required for the loop exceeds the block gas limit, causing the transaction to fail.
**Outcome**:
- The `distributeRewards` function becomes unusable, effectively creating a Denial of Service (DoS) condition. Legitimate users attempting to distribute rewards will face transaction failures, disrupting the protocol's intended operations.

Impact

This attack results in a Denial of Service (DoS) for critical protocol functions. Users cannot receive rewards, administrators cannot perform necessary operations across all vaults, and the protocol's functionality is severely impaired.

Tools Used

manual code review

Recommendations

Implement pagination in functions that iterate over vaults. You could use mappings instead of arrays for vault storage and implement a mechanism to remove inactive or empty vaults.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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