Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L161-L168 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L118-L147 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L89-L116 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L46-L79

Vulnerability details

Bug description

In order for an operator to deploy a new restaking vault, he must call Core::deployVaults(), passing in configuration of the vault and the implementation to use as the parameters.

The function calls the CoreLib::deployVaults() passing in parameters supplied by the operator.

Core.sol#L167

vaults = _self().deployVaults(msg.sender, vaultImpl, vaultConfigs);

deployVaults() function of the CoreLib library will validate that the implementation and underlying tokens are allowlisted in the system and then will call createVault() function.

CoreLib.sol#L77-L87

function validateVaultConfigs(
    Storage storage self,
    VaultLib.Config[] calldata vaultConfigs,
    address implementation
) public view {
    if (
        !(implementation == address(0) ||
            isVaultImplAllowlisted(self, implementation))
    ) {
        revert VaultImplNotAllowlisted();
    }
    for (uint256 i = 0; i < vaultConfigs.length; i++) {
        if (self.assetSlashingHandlers[vaultConfigs[i].asset] == address(0))
            revert AssetNotAllowlisted();
    }
}

After cloning a vault, createVault() function will initialize it.

CoreLib.sol#L89-L116

IKarakBaseVault vault = cloneVault(salt);
vault.initialize(
    address(this),
    operator,
    depositToken,
    name,
    symbol,
    extraData
);

As can be seen, the extraData parameter is not validated, thus an operator creating a new vault can pass any arbitrary values. This is problematic, because during NativeVault's initialization, extraData parameter is used to set the value of the manager, slashStore and the nodeImplementation in the NativeVault.

NativeVault.sol#L46-L79

(address manager, address slashStore, address nodeImplementation) = abi
    .decode(_extraData, (address, address, address));
if (
    manager == address(0) ||
    slashStore == address(0) ||
    nodeImplementation == address(0)
) revert ZeroAddress();
_grantRoles(manager, Constants.MANAGER_ROLE);

self.slashStore = slashStore;
self.nodeImpl = nodeImplementation;

Essentially an operator, while not being a trusted entity in the system, can severely escalate his privileges. By setting the address holding a trusted role of the manager to an address he controls, an operator can change the implemenation of every NativeNode of his NativeVault introducing an additional withdraw function that will transfer ETH to his address stealing from the stakers. This is possible because changeNodeImplementation() of the NativeVault can be called by an address with a manager role.

NativeVault.sol#L83-L92

function changeNodeImplementation(
    address newNodeImplementation
)
    external
    onlyOwnerOrRoles(Constants.MANAGER_ROLE)
    whenFunctionNotPaused(Constants.PAUSE_NATIVEVAULT_NODE_IMPLEMENTATION)
{
    if (newNodeImplementation == address(0)) revert ZeroAddress();
    _state().nodeImpl = newNodeImplementation;
    emit UpgradedAllNodes(newNodeImplementation);
}

Additionally, by setting an address of the slashStore to an address not equal to the slashingHandler of the underlying asset, an operator can block any slashing of his NativeVault. When slashAssets() will be called on a vault, the slashingHandler set in the Core for the underlying asset will be used as the parameter.

SlasherLib.sol#L134-L139

IKarakBaseVault(queuedSlashing.vaults[i]).slashAssets(
    queuedSlashing.earmarkedStakes[i],
    self.assetSlashingHandlers[
        IKarakBaseVault(queuedSlashing.vaults[i]).asset()
    ]
);

During the call to slashAssets(), if the slashStore is not equal to the slashingHandler of the underlying asset, the transaction will revert.

NativeVault.sol#L308

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

Impact

Operator can severely escalate his privileges, steal staker's funds and block any slashing of his NativeVault.

Recommended Mitigation

Add an additional validation of the extraData parameter passed by the operator during vault creation. Alternatively, pull the manager and the slashStore addresses from the Core contract during NativeVault initialization and only let the manager set the nodeImplementation, since currently it is also entirely operator supplied.

Assessed type

Other

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