Insufficient validation leads to overwriting Existing Slash Handlers

[c4-bot-10]

Lines of code

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

Vulnerability details

Intro

The validation in the allowlistAssets function is not implemented correctly which can lead to serious issues

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

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

This validation is insufficient because:

1. It only checks that the asset and slashing handler addresses are not zero addresses.

2. It doesn't verify if the asset is already allowlisted or if it's being reassigned a different slashing handler.

3. Most importantly, it lacks a crucial check that is present in the validateVaultConfigs function:

if (self.assetSlashingHandlers[vaultConfigs[i].asset] == address(0)) revert AssetNotAllowlisted();

The validateVaultConfigs function checks if an asset has a slashing handler assigned, which implies that it's allowlisted. However, the allowlistAssets function doesn't perform any check to see if the asset is already allowlisted or if it's safe to update its slashing handler.

Impact

The allowlistAssets function will silently overwrite existing slashing handlers for assets. There's no check to see if an asset already has a handler.

Mitigation

1. Verify if the asset is already allowlisted and decide whether to allow updates or revert if an asset is being re-allowlisted.
2. If updates are allowed, emit an event to log the change of slashing handler for an asset.

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

        address currentHandler = self.assetSlashingHandlers[assets[i]];
        if (currentHandler != address(0) && currentHandler != slashingHandlers[i]) {
            emit SlashingHandlerUpdated(assets[i], currentHandler, slashingHandlers[i]);
        } else if (currentHandler == address(0)) {
            emit AssetAllowlisted(assets[i], slashingHandlers[i]);
        }

        self.assetSlashingHandlers[assets[i]] = slashingHandlers[i];
    }
}

Assessed type

Invalid Validation