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

0 stars 0 forks source link

Operator can evade slashing by using unregistered slashHandler address #84

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/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L308

Vulnerability details

Impact

An operator can evade slashing from the core and make the call to revert whenever that happens. This is because , the operator has put an address that should not have been allowed there.

Bug Description

During the vault deployments the vaultConfigs's extraData was not sanitized enough.

function deployVaults(VaultLib.Config[] calldata vaultConfigs, address vaultImpl)
        external
        whenFunctionNotPaused(Constants.PAUSE_CORE_DEPLOY_VAULTS)
        nonReentrant
        returns (IKarakBaseVault[] memory vaults)
    {
>        vaults = _self().deployVaults(msg.sender, vaultImpl, vaultConfigs);
    }

This oversight allows the operator to be able to set up the slashHandler or the slashStore that when an operator is slashed gets to hold the slashed assets and to decide what to do with it as per the protocol.

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

   for (uint256 i = 0; i < queuedSlashing.vaults.length; i++) {
135            IKarakBaseVault(queuedSlashing.vaults[i]).slashAssets(
                  queuedSlashing.earmarkedStakes[i],

   self.assetSlashingHandlers[IKarakBaseVault(queuedSlashing.vaults[i]).asset()]
            );
        }

This will now allow the operator to evade slashing, because whenever the finalizeSlashing is called, the function will revert.

This is because, the operator has set up an unrecognized address as the handler/slashStore.

This is where the call from the core will revert

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L308C9-L308C72

if (slashingHandler != self.slashStore) revert NotSlashStore();

Tools Used

Manual review

Recommended Mitigation Steps

I recommend that the slash handler be sanitized or supplied independently thats sourced from the the AllowListed where the slashHandler is the one that has been verified already through the allowlistAssets function.

        mapping(address asset => address slashingHandler) assetSlashingHandlers;

Assessed type

Invalid Validation

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory

ABDuullahi commented 2 months ago

Hello @MiloTruck

This issue i believe can be partially duplicated with issue #049 seeing i have a confirmed issue in that family and also given the fact that this issue identified the maximum impact and the mitigation seems feasible.

Thanks

MiloTruck commented 1 month ago

Not sure what the issue is here, this is an exact duplicate of https://github.com/code-423n4/2024-07-karak-findings/issues/55.

https://github.com/code-423n4/2024-07-karak-findings/issues/49 is about the slashing handler for an asset being changed.