User can request update of the vault stake and then unregister before finalization

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L189

Vulnerability details

Impact

In the current implementation of the Core, the operator has to be registered to a chosen DSS to be able to stake the vault to it. However, current functionality is mostly 2step where first the operation has to be requested and then finalized after some cooldown. The problem is that when user requests staking to DSS, the function checks if it's registered or not but when it finalizes, it does not check the registration status so the user can effectively front-run the finalization transaction and unregister immediately as the unregister() function will show there are no vaults of the operator that are staked to this particular DSS.

Proof of Concept

1. Let's say the user calls requestUpdateVaultStakeInDSS(), the function checks the registration status of the operator:

https://github.com/code-423n4/2024-07-karak/blob/main/src/Core.sol#L138

 self.checkIfOperatorIsRegInRegDSS(operator, vaultStakeUpdateRequest.dss);

2. Then some time should pass before the request can be finalized called MIN_STAKE_DELAY:

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L93-95

  if (queuedStakeUpdate.startTimestamp + Constants.MIN_STAKE_UPDATE_DELAY > block.timestamp) {
            revert OperatorStakeUpdateDelayNotPassed();
        }

3. Additionally, when user unregisters from DSS, the function unregisterOperatorFromDSS checks whether there are not any vaults staked to this particular DSS:

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L190

 if (vaults.length != 0) revert AllVaultsNotUnstakedFromDSS();

4. The vault is only added to the vaults staked to DSS structure only when request is finalized:

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L103-109

  function updateVaultStakeInDSS(State storage operatorState, address vault, IDSS dss, bool toStake) internal {
        if (toStake) {
            operatorState.vaultStakedInDssMap[dss].add(vault);
        } else {
            operatorState.vaultStakedInDssMap[dss].remove(vault);
        }
    }

5. So the issue lies in not checking whether operator is registered to DSS when finalizing request:

https://github.com/code-423n4/2024-07-karak/blob/main/src/entities/Operator.sol#L89-101

  function validateQueuedStakeUpdate(State storage operatorState, QueuedStakeUpdate memory queuedStakeUpdate)
        internal
        view
    {
        if (queuedStakeUpdate.startTimestamp + Constants.MIN_STAKE_UPDATE_DELAY > block.timestamp) {
            revert OperatorStakeUpdateDelayNotPassed();
        }
        if (
            calculateRoot(queuedStakeUpdate) != operatorState.pendingStakeUpdates[queuedStakeUpdate.updateRequest.vault]
        ) {
            revert InvalidQueuedStakeUpdateInput();
        }
    }

The function only checks if the root of the structure corresponds to the one in the pending updates and if the sufficient amount of time has passed. This can effectively lead to a situation where an operator can be staked to DSS while being unregistered from it. It's considered as a deviation from the spec as the vault can potentially earn rewards and cannot be slashed.

Tools Used

Manual review.

Recommended Mitigation Steps

Check if operator is registered in DSS when finalizing request stake update.

Assessed type

Other

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Out of scope

[rodiontr]

Hi @MiloTruck I am not sure if this is against the rules but this issue was in the audit report and said to be "resolved". So I was reporting it having this in my mind. I think if there are audit issues in the past audit reports, it seems pretty logical that wardens should watch for the fixes in the case of something and in this case the issue was marked as "resolved" but was not actually resolved

[MiloTruck]

For this contest, issues that have been reported in previous audits before were publicly announced to be OOS:

https://discord.com/channels/810916927919620096/1260968277035843607/1267792210863067221

To be fair to everyone who did not submit after seeing this announcement, I'm abiding by this and invalidating all previously reported issues.

Feel free to open an issue in the org repo if you think previously reported issues should be considered in-scope, but unfortunately, I can't change a decision that was already made in the past.