Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Valid

Inconsistent vault membership tracking in OperatorVCS contract

Summary

The removeVault function in this contract does not update the vaultMapping when removing a vault, which can lead to potential inconsistencies between the vaults array and the vaultMapping. This will result in removed vaults still being considered as members of the strategy.

Vulnerability Details

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorVCS.sol#L304-L330

This issue is present in the removeVault function of the OperatorVCS contract. After removing a vault from the vaults array, the function does not update the vaultMapping to reflect this change.

function removeVault(uint256 _queueIndex) public {
// ... (earlier code)
for (uint256 i = index; i < numVaults - 1; ++i) {
vaults[i] = vaults[i + 1];
}
vaults.pop();
// Missing: vaultMapping[vault] = false;
// ... (remaining code)
}

The vaultMapping is used in other parts of the contract to verify vault membership, such as in the withdrawOperatorRewards function:

function withdrawOperatorRewards(
address _receiver,
uint256 _amount
) external returns (uint256) {
if (!vaultMapping[msg.sender]) revert SenderNotAuthorized();
// ... (remaining code)
}

Impact

This vulnerability could lead to the following issues:

Removed vaults might still be able to call functions restricted to active vaults, such as withdrawOperatorRewards.
Inconsistency between the vaults array and vaultMapping could lead to confusion and potential exploitation.
It may interfere with proper accounting and management of vaults within the strategy.

Tools Used

manual review

Recommendations

consider adding a line to set the vault's mapping to false after removing it from the vaults array:

vaults.pop();
vaultMapping[vault] = false; // Add this line
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`removeVault` does not update `vaultMapping`

Support

FAQs

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