Lines of code

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

Vulnerability details

Impact

The operator can have the ability to change the implementation of the contract, potentially to a malicious one where assets can be drained.

The operator can also denied withdrawal by pausing the withdraw function of the nativeNode

Bug Description

As per the karak implementation and according to the readMe, one of the trusted roles is the MANAGER role, where it has the ability to pause some certain functions in case of an emergency

To ensure pauses are prompt, 4 team wallets are given a manager role which strictly enables them to pause the Karak contracts in an emergency situation, but hold no other privileges.

and as per as the conversation i had with the sponsor, the operator should just have the ability to stake and unstake its vault

So there's no manager of vault, only owner, and it's the core not operator. Operator just has the functionality to stake/unstake it's vault.

But due to the way the vault deployment has been implemented,

    /// @notice Deploys a new restaking vault with the given configuration to an operator
    /// @dev Emits a DeployedVault event for each vault deployed
    /// @dev It is meant to be permissionless since the setting of the standard vault implementation is permissioned
    /// @param vaultConfigs: The configurations of the vaults to deploy
    /// @param vaultImpl: The address of the vault implementation to use
    /// @return vaults {IKarakBaseVault[]} The addresses of the deployed vaults
    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);
    }

an operator can set the MANAGER of his vault during deployments by utilizing the extraData of the config struct which would later be decoded and used during the initialization to set up the vault, this can be native or the normal vault.

This would mean that the operator can choose who the manager is going to be, potentially allowing him to call some certain restricted function that include

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

 function pauseNode(INativeNode node, uint256 map) external onlyRolesOrOwner(Constants.MANAGER_ROLE) {
        node.pause(map);
    }

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

function unpauseNode(INativeNode node, uint256 map) external onlyRolesOrOwner(Constants.MANAGER_ROLE) {
        node.unpause(map);
    }

or the changeNodeImplementation that will allow him to change the implementation, potentially to a malicious one

Tools Used

Manual review

Recommended Mitigation Steps

This can be mitigated by either removing the extra data or by validating the supply of the address.

Assessed type

Access Control

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

Operator can become one of the trusted roles #85