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

Flaw in VaultDepositController's withdraw Parameter Handling

0bingo76

Summary

The withdraw function in the VaultDepositController contract contains a potential bug related to the handling of the _data parameter. Specifically, if `_data` is empty or improperly formatted, it can lead to an out-of-bounds error during the decoding process, resulting in a confusing error message for users.

Vulnerability Details

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

if (vaultIds[0] != group.withdrawalIndex) revert InvalidVaultIds();

This line is where the bug manifests. The issue arises because the code attempts to access vaultIds[0] without first checking if vaultIds is non-empty. If the _data parameter is empty or improperly formatted, the abi.decode call will result in an empty array, and attempting to access the first element (vaultIds[0]) will cause an out-of-bounds error.

To prevent this, you should add a check to ensure that vaultIds is not empty before accessing its elements.

- `withdraw(uint256 _amount, bytes calldata _data)`
- The function decodes `_data` into an array of `vaultIds` without checking if the array is empty. If `_data` is empty or does not contain valid vault IDs, the `abi.decode` call will revert with an out-of-bounds error when attempting to access `vaultIds[0]`.
- The issue occurs before the check `if (vaultIds[0] != group.withdrawalIndex)`.

Impact

The function may not handle all valid input scenarios correctly, potentially leading to failed withdrawals and user dissatisfaction.

Alice and Bob are users of the DeFi platform that utilizes the `VaultDepositController` contract.
​
1. Alice deposits tokens into the vault and decides to withdraw them later.
- She calls the `withdraw` function with a valid `_amount` and correctly formatted `_data`.
- The transaction succeeds, and Alice receives her tokens.
​
2. Bob, who is less familiar with the platform, also wants to withdraw his tokens.
- He calls the `withdraw` function with a valid `_amount` but forgets to include any vault IDs in the `_data` parameter.
- Bob's transaction unexpectedly reverts with a cryptic out-of-bounds error.
- Confused, Bob contacts customer support, unsure why his withdrawal failed.
​
3. The development team investigates Bob's failed transaction:
- They realize that the function doesn't gracefully handle empty `_data` input.
- The team identifies that the `abi.decode` operation is attempting to access an element in an empty array, causing the out-of-bounds error.

Tools Used

manual

Recommendations

Before accessing `vaultIds[0]`, add a check to ensure that `vaultIds` is not empty.

if (vaultIds.length == 0) revert InvalidVaultIds();
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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