code-423n4 / 2024-07-karak-findings

0 stars 0 forks source link

finalizeUpdateVaultStakeInDSS() can be called after an operator has unregistered from the DSS #71

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

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

Vulnerability details

Vulnerability Details

Core::requestUpdateVaultStakeInDSS() allows only operators that are registered to a certain DSS to stake/unstake vaults to it. However this can be bypassed because Core::finalizeUpdateVaultStakeInDSS() doesn't check if the operator is still registered to the certain DSS, thus an operator can unregister from the DSS between the requested update and the finalization. Let's see how this is a problem:

  1. An operator is registered to a certain DSS and calls Core::requestUpdateVaultStakeInDSS to stake a vault.
  2. Now there is a 9 days delay until someone can execute the finalization of the update.
  3. The operator calls Core::unregisterOperatorFromDSS(). Since the state is not updated from the requested vault update and the operator still doesn't have any vaults staked to the DSS, the transaction executes successfully and now the operator is no longer staked to the DSS.
  4. The rest of MIN_STAKE_UPDATE_DELAY passes and now either the operator or a random person calls Core::finalizeUpdateVaultStakeInDSS() to stake the vault to the DSS. Since a check is missing here to make sure that the operator is still registered to the DSS, the transaction executes, the state is updated and now this vault is staked to the DSS.
  5. Now the above-mentioned vault cannot be unstaked, because in the Core::requestUpdateVaultStakeInDSS there is a check if the operator is registered to the DSS, but he is not. The DSS cannot request slashing because there is a check to see if the operator is registered to the DSS, this means that the operator can do whatever he wants and still be left unpunished by the DSS. Core::operatorState[operator].vaultStakedInDssMap[dss] will contain the inaccessible vault.

Impact

Does an incorrect state update to the vaultStakedInDssMap mapping which could lead to a lot of problems including unfair distribution of rewards, inability of slashing.

Tools Used

Manual Review

Recommended Mitigation Steps

In Operator::validateAndUpdateVaultStakeInDSS() check if the operator is still registered to the DSS.

function validateAndUpdateVaultStakeInDSS(CoreLib.Storage storage self, QueuedStakeUpdate memory queuedStakeUpdate)
        external
    {
        State storage operatorState = self.operatorState[queuedStakeUpdate.operator];
+       checkIfOperatorIsRegInRegDSS(self, queuedStakeUpdate.operator, queuedStakeUpdate.updateRequest.dss);
        validateQueuedStakeUpdate(operatorState, queuedStakeUpdate);
        updateVaultStakeInDSS(
            operatorState,
            queuedStakeUpdate.updateRequest.vault,
            queuedStakeUpdate.updateRequest.dss,
            queuedStakeUpdate.updateRequest.toStake
        );
        delete operatorState.pendingStakeUpdates[queuedStakeUpdate.updateRequest.vault];
        IDSS dss = queuedStakeUpdate.updateRequest.dss;
        HookLib.callHookIfInterfaceImplemented({
            dss: dss,
            data: abi.encodeWithSelector(dss.finishUpdateStakeHook.selector, msg.sender),
            interfaceId: dss.finishUpdateStakeHook.selector,
            ignoreFailure: true,
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

Assessed type

Invalid Validation

c4-judge commented 2 months ago

MiloTruck marked the issue as unsatisfactory: Out of scope