Lines of code

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

Vulnerability details

Impact

Due to a missing check on the slashStore address that is included in the encoded extraData in the configuration struct passed into Core.deployVaults(), an Operator can easily pass in slash store contract that is not that of the Protocol and move all slashed assets away.

Proof of Concept

Let us consider a Vault(one deployed with erc20s tokens as asset)

When an operator calls Core.deployVaults() with any token passed in as asset, flow is passed to _self().deployVaults(msg.sender, vaultImpl, vaultConfigs), where validateVaultConfigs() validates that each token to be used as an asset in each vault has a corresponding slashHandler contract.

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();
    }
}

Also, when we check the finalization logic, it was observed that, the slashHandler address that was utilised is the one set by protocol admin in storage state

    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) {
            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()]
            );
        }
...code
    }

Set by protocol admin

  function allowlistAssets(address[] memory assets, address[] memory slashingHandlers)
        external
   @>     onlyRolesOrOwner(Constants.MANAGER_ROLE) //PROTOCOL_ADMIN
    {
        _self().allowlistAssets(assets, slashingHandlers);
        emit AllowlistedAssets(assets);
    }

      function allowlistAssets(Storage storage self, address[] memory assets, address[] memory slashingHandlers)
        external
    {
        if (assets.length != slashingHandlers.length) revert LengthsDontMatch();
        for (uint256 i = 0; i < assets.length; i++) {
            if (slashingHandlers[i] == address(0) || assets[i] == address(0)) revert ZeroAddress();
    @>        self.assetSlashingHandlers[assets[i]] = slashingHandlers[i];
        }
    }

But when deploying a NativeVault, there is no where the slash store contract address in the encoded data verified to be protocol owned, the only check was in the initialisation for zero address

 function initialize(
        address _owner,
        address _operator,
        address _depositToken,
        string memory _name,
        string memory _symbol,
        bytes memory _extraData
    ) external initializer {
        _initializeOwner(_owner);
        __Pauser_init();

        if (_depositToken != Constants.DEAD_BEEF) revert DepositTokenNotAccepted();

        (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);

        NativeVaultLib.Storage storage self = _state();
        VaultLib.Config storage config = _config();

        config.asset = _depositToken;
        config.name = _name;
        config.symbol = _symbol;
        config.decimals = 18;
        config.operator = _operator;
        config.extraData = _extraData;

  @>    self.slashStore = slashStore;
        self.nodeImpl = nodeImplementation;

        emit NativeVaultInitialized(_owner, manager, _operator, slashStore);
    }

And this is where slashed funds are sent to

    function _transferToSlashStore(address nodeOwner) internal {
        NativeVaultLib.Storage storage self = _state();
        NativeVaultLib.NativeNode storage node = self.ownerToNode[nodeOwner];

        // slashed ETH = total restaked ETH (node + beacon) - share price equivalent ETH
        uint256 slashedAssets = node.totalRestakedETH - convertToAssets(balanceOf(nodeOwner));

        // sweepable ETH = min(ETH available on node address, total slashed ETH)
        uint256 slashedWithdrawable = Math.min(node.nodeAddress.balance, slashedAssets);

        // withdraw sweepable ETH to slashStore
@>       INativeNode(node.nodeAddress).withdraw(self.slashStore, slashedWithdrawable);

        // update total restaked ETH available (node + beacon)
        node.totalRestakedETH -= slashedWithdrawable;

        // update withdrawable credited ETH available on the node only when node balance
        // decreases to less than that of the credited node ETH
        if (node.nodeAddress.balance < node.withdrawableCreditedNodeETH) {
            node.withdrawableCreditedNodeETH = node.nodeAddress.balance;
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

Validate the slash store address in the encoded data to be protocol controlled if an operator is attempting to deploy a NativeVaults.

Also, validates the Manager and Implementation address.

Assessed type

Invalid Validation

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

An Operator can deploy a vault with a controlled slash store to receive slashed Eth from NativeVault #87