DOSing of NativeVault deployment.

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L42 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L85 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/CoreLib.sol#L72

Vulnerability details

Impact

When deploying a NativeVault, asset address in the configuration struct(Operator input) can be zero according to NativeVault initialization function

 /// @notice Initializes the vault
    /// @param _owner The owner of the NativeVault, which should be Core in most cases.
    /// @param _operator The operator of the vault (usually the caller of the deployVault function on Core which triggers this).
@>  /// @param _depositToken Disregarded for NativeVault. Can be address(0).
    /// @param _name The name of the vault
    /// @param _symbol The symbol of the vault
    /// @param _extraData Serialized bytes consisting {address slashStore, address nodeImplementation}
    function initialize(
        address _owner,
        address _operator,
        address _depositToken,
        string memory _name,
        string memory _symbol,
        bytes memory _extraData
    ) external initializer {

Also, protocol admin set allowlist(tokens that can be used as vault asset), which include setting up the slash Handler address, in the settings, zero address cannot be set up

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

From the above snippet, it shows that no slashHandler is set for address(0). This is still making sense, since when deploying a NativeVault that accept address(0) as asset address the slash store is encoded as extraData

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

The cause of DOS here lies in the validation check when deploying vault. When a NativeVault is being deployed and asset address is zero, the function reverts

Proof of Concept

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

From previous observation, no slashHandler is set for address(0) in allowlistAssets(), this means if an Operator passes in address(0) as asset address with the aim of deploying NativeAsset, the deployment will revert!

Tools Used

Manual review.

Recommended Mitigation Steps

Decode the extraData and validate that slash store address is not zero if the slashhandler is zero.

    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++) {
+++ (address manager, address slashStore, address nodeImplementation) =
            abi.decode(vaultConfigs[i].extraData], (address, address, address));

---    if (self.assetSlashingHandlers[vaultConfigs[i].asset] == address(0)) revert AssetNotAllowlisted();
+++    if (self.assetSlashingHandlers[vaultConfigs[i].asset] == address(0) && slashStore == address(0) ) revert AssetNotAllowlisted();
        }
    }

Assessed type

DoS

[c4-judge]

MiloTruck marked the issue as not a duplicate

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Invalid

[MiloTruck]

There is no issue here.

If an operator calls deployVaults() to deploy a native vault with slashStore == address(0), it will already revert.