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

0 stars 0 forks source link

Invalid slash store validation makes it possible for operator to dodge slashing #75

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#L46-L79 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/entities/CoreLib.sol#L118-L147 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/NativeVault.sol#L299-L318 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L134-L139 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L308

Vulnerability details

Impact

In the Core.sol file the operator has the ability to create native vaults which they can later register to a DSS which will have the ability to slash the operator if they are not cooperating. However when creating a vault the operator can create a vault in such way that when the DSS tries to slash their asset it will revert. The operator can achieve that because when creating a native vault the operator has the ability to specify arbitrary _extraData from which the slashStore is decoded and which is later assigned to self.slashStore in the native vault as can be seen here: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L46-L79

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

which is called from the https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L106-L107

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

which is called from here https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L132-L147

        for (uint256 i = 0; i < vaultConfigs.length; i++) {
            IKarakBaseVault vault = createVault(
                self,
                operator,
                vaultConfigs[i].asset,
                vaultConfigs[i].name,
                vaultConfigs[i].symbol,
                vaultConfigs[i].extraData,
                implementation
            );
            vaults[i] = vault;
            self.operatorState[operator].addVault(vault);
            emit DeployedVault(operator, address(vault), vaultConfigs[i].asset);
        }
        return vaults;
    }

and which is initially called by the operator in the core contract as can be seen here https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/Core.sol#L161-L168

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

After the native vault is created with a non standard slashStore address when the DSS tries to finalize the slashing the slashAssets will be called in the following way: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/SlasherLib.sol#L134-L139

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

where the slashStore is taken from assetSlashingHandlers of the core which just assigns a slashStore to each allowlisted asset in the core. When the function is called with the slashStore that the core has assigned to the asset but in the native vault the slashStore is different the transaction will revert because of the following check in the native vault: https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L308

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

As such the DSS would be unable to slash the operator's vault and the owner (the core in this case) has no ability to change the slashStore of the vault after it was initialized as such the vault would become invulnerable to slashing as long as it exists.

Proof of Concept

Add the following test in the nativeVault.t.sol and run forge test --match-test test_create_bad_vault

    function test_create_bad_vault() public {
        address nativeNodeImpl = address(new NativeNode());
        VaultLib.Config[] memory badVaultConfigs = new VaultLib.Config[](1);
        badVaultConfigs[0] = VaultLib.Config({
            asset: Constants.DEAD_BEEF,
            decimals: 18,
            operator: operator,
            name: "NativeTestVault",
            symbol: "NTV",
            extraData: abi.encode(address(manager), address(operator), address(nativeNodeImpl))
        });
        vm.startPrank(operator);
        IKarakBaseVault[] memory vaults = core.deployVaults(badVaultConfigs, address(0));
        vm.stopPrank();
    }

Tools Used

Manual

Recommended Mitigation Steps

Set the slashStore automatically when creating a vault based on the selected asset as per asset allowlist.

Assessed type

Invalid Validation

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory