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

0 stars 0 forks source link

The DSS can slash an operator even after they change their vault to be staked to another DSS #103

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L94-L124 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L126-L151 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L220-L231 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L248-L256

Vulnerability details

When the operator stakes their vault to the DSS the DSS gains the ability to slash the operator if they are not performing the tasks as it is needed. That ability is used to punish bad operators and the DSS should only be able to slash vaults that are assigned to them and no other vault. However due to the way that slashing is being finalized the DSS can slash the operator even after they have unstaked their vault from the DSS. That issue happens due to the following flow missing an important validation: The DSS decides to request a slashing of the operator's vault by calling the requestSlashing function in the Core.sol which checks if the operator is registered to the DSS and their vault is staked to that DSS: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L220-L231

    function requestSlashing(SlasherLib.SlashRequest calldata slashingRequest)
        external
        whenFunctionNotPaused(Constants.PAUSE_CORE_REQUEST_SLASHING)
        nonReentrant
        returns (SlasherLib.QueuedSlashing memory queuedSlashing)
    {
        IDSS dss = IDSS(msg.sender);
        CoreLib.Storage storage self = _self();
        self.checkIfOperatorIsRegInRegDSS(slashingRequest.operator, dss);
        queuedSlashing = self.requestSlashing(dss, slashingRequest, self.nonce++);
        emit RequestedSlashing(address(dss), slashingRequest);
    }

then the request is decoded in the SlasherLib.sol and stored in the slashingRequests for future use: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L94-L124

    function requestSlashing(
        CoreLib.Storage storage self,
        IDSS dss,
        SlashRequest memory slashingMetadata,
        uint256 nonce
    ) external returns (QueuedSlashing memory queuedSlashing) {
        validateRequestSlashingParams(self, slashingMetadata, dss);
        uint256[] memory earmarkedStakes = fetchEarmarkedStakes(slashingMetadata);
        queuedSlashing = QueuedSlashing({
            dss: dss,
            timestamp: uint96(block.timestamp),
            operator: slashingMetadata.operator,
            vaults: slashingMetadata.vaults,
            earmarkedStakes: earmarkedStakes,
            nonce: nonce
        });
        self.slashingRequests[calculateRoot(queuedSlashing)] = true;
        self.operatorState[slashingMetadata.operator].nextSlashableTimestamp[dss] =
            block.timestamp + Constants.SLASHING_COOLDOWN;
        HookLib.callHookIfInterfaceImplemented({
            dss: dss,
            data: abi.encodeWithSelector(
                dss.requestSlashingHook.selector, slashingMetadata.operator, slashingMetadata.slashPercentagesWad
            ),
            interfaceId: dss.requestSlashingHook.selector,
            ignoreFailure: true,
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

After some time the DSS can call the finalizeSlashing function which will validate whether the root of that request is stored in the mapping and whether the veto period of 2 days has passed and if so slash the vault of the operator as can be seen in those two functions: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L248-L256

    function finalizeSlashing(SlasherLib.QueuedSlashing memory queuedSlashing)
        external
        nonReentrant
        whenFunctionNotPaused(Constants.PAUSE_CORE_FINALIZE_SLASHING)
    {
        _self().finalizeSlashing(queuedSlashing);

        emit FinalizedSlashing(msg.sender, queuedSlashing);
    }

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L126-L151

    function finalizeSlashing(CoreLib.Storage storage self, QueuedSlashing memory queuedSlashing) external {
        bytes32 slashRoot = calculateRoot(queuedSlashing);
        if (!self.slashingRequests[slashRoot]) revert InvalidSlashingParams();
        if (queuedSlashing.timestamp + Constants.SLASHING_VETO_WINDOW > block.timestamp) { //@audit no max time or active vault checks, the DSS can start a slashing and wait until the operator has staked their vault to another DSS and then once again slash it even though it doesn't belong to the DSS
            revert MinSlashingDelayNotPassed();
        }
        delete self.slashingRequests[slashRoot];

        for (uint256 i = 0; i < queuedSlashing.vaults.length; i++) {
            IKarakBaseVault(queuedSlashing.vaults[i]).slashAssets(
                queuedSlashing.earmarkedStakes[i],
                self.assetSlashingHandlers[IKarakBaseVault(queuedSlashing.vaults[i]).asset()]
            );
        }
        IDSS dss = queuedSlashing.dss;

        HookLib.callHookIfInterfaceImplemented({
            dss: dss,
            data: abi.encodeWithSelector(dss.finishSlashingHook.selector, queuedSlashing.operator),
            interfaceId: dss.finishSlashingHook.selector,
            ignoreFailure: true,
            hookCallGasLimit: self.hookCallGasLimit,
            supportsInterfaceGasLimit: self.supportsInterfaceGasLimit,
            hookGasBuffer: self.hookGasBuffer
        });
    }

Impact

However as can be seen here the library doesn't check wether the vault is still staked to the DSS as such the DSS can request the slashing, wait an arbitrarily long time and even after the operator has unstaked their vault from the DSS they can finalize the slashing thus punishing the operator even after they have unstaked or even registered to another DSS.

Proof of Concept

  1. Create a Vault
  2. Register to a DSS
  3. Stake the vault to the DSS
  4. Request slashing as the DSS
  5. Wait more than 2 days
  6. Unstake the vault or even register to another DSS
  7. Finalize the slashing and slash the operator's funds

Tools Used

Manual review

Recommended Mitigation Steps

Check whether the operator is still staked to the DSS when finalizing the slashing.

Assessed type

Invalid Validation

c4-judge commented 1 month ago

MiloTruck marked the issue as satisfactory

c4-judge commented 1 month ago

MiloTruck changed the severity to 3 (High Risk)