In the OperatorVCS contract, the queueVaultRemoval and removeVault functions are responsible for handling the removal of vaults. A race condition exists due to the contract's reliance on external state conditions when determining whether a vault should be queued for removal. Specifically, a vault can be queued for removal based on the status of an operator, but if the operator’s status changes before the vault is actually removed, it can result in unintended behavior such as removing an active vault. This vulnerability can be exploited to disrupt the proper functioning of vault management within the contract.
The vault removal process in OperatorVCS has two steps: queuing the vault for removal in the queueVaultRemoval function and subsequently removing it in removeVault. The removal process relies on external state conditions such as the operator's status in the Chainlink staking contract. However, a race condition can occur if the status of the operator (e.g., whether the operator is removed from the staking contract) changes between the queuing and removal steps. This could allow an attacker or even normal users to exploit the system by triggering vault removal while the operator is still active or has changed status.
Race condition:
The function queueVaultRemoval queues the vault for removal based on the external IVault(vault).isRemoved() status. This status check determines whether the vault is eligible for removal based on the operator's status in the staking contract.
However, the operator’s status can change after the vault is queued but before it is actually removed. The contract relies on the state of vaultsToRemove and doesn’t re-check the operator's status during the removeVault step.
If the operator is reactivated or the vault becomes active again, the vault could still be removed, causing unintended behavior and potentially disrupting the staking system.
PoC scenario:
Assume there is an active vault associated with an operator that becomes temporarily inactive (e.g., removed from the staking contract).
A user or attacker calls queueVaultRemoval to add the vault to the removal queue. The operator’s status is checked and the vault is queued.
Before removeVault is called, the operator’s status changes, and the vault becomes active again.
When removeVault is executed, the vault is removed despite being active, potentially leading to incorrect behavior such as loss of funds or disruption in staking operations.
Here is a test that can be run using Hardhat to demonstrate the race condition:
The test will pass, showing that the vault was removed even though the operator was reactivated after being queued for removal, demonstrating the race condition.
The race condition allows for the unintended removal of vaults that have become active again after being queued for removal. This could result in:
Active vaults being removed, causing operational disruptions.
Potential loss of staking rewards for operators and users.
Unpredictable behavior in the vault management process.
Manual review.
In the removeVault function, add an additional check to ensure that the vault is still in a removed state before allowing removal. This will prevent vaults that have been reactivated from being removed.
Example:
Instead of relying on external states that can change between function calls, consider bundling the removal process into a single atomic transaction where the vault is checked and removed in one step, reducing the risk of state changes between calls.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.