Lack of Deprecation Check in updateVersionsByAsset

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/registry/AssetPluginRegistry.sol#L51

Vulnerability details

Vulnerability Details

The AssetPluginRegistry contract manages the validity of assets across different versions. It includes a deprecation mechanism, but this is not consistently applied across all functions.

The updateVersionsByAsset function allows updating the validity of an asset for multiple versions. However, it doesn't check if the asset has been deprecated before making these updates.

Code Snippet

function updateVersionsByAsset(
    address _asset,
    bytes32[] calldata _versionHashes,
    bool[] calldata _validities
) external {
    // ... (existing checks)

    for (uint256 i = 0; i < _versionHashes.length; ++i) {
        bytes32 versionHash = _versionHashes[i];
        if (address(versionRegistry.deployments(versionHash)) == address(0)) {
            revert AssetPluginRegistry__InvalidVersion();
        }

        _isValidAsset[versionHash][_asset] = _validities[i];

        emit AssetPluginRegistryUpdated(versionHash, _asset, _validities[i]);
    }
}

Impact

This bug allows deprecated assets to be re-validated, which contradicts the purpose of the deprecation mechanism. It can lead to inconsistent state and potential security risks if deprecated assets are unknowingly reactivated.

Scenario

1. An asset is deprecated using the deprecateAsset function.
2. Later, updateVersionsByAsset is called for this asset, setting it as valid for some versions.
3. The asset is now valid for these versions, despite being deprecated.
4. The isValidAsset function will still return false due to the deprecation check, leading to an inconsistent state.

Fix

Add a deprecation check at the beginning of the updateVersionsByAsset function:

function updateVersionsByAsset(
    address _asset,
    bytes32[] calldata _versionHashes,
    bool[] calldata _validities
) external {
    if (!roleRegistry.isOwner(msg.sender)) {
        revert AssetPluginRegistry__InvalidCaller();
    }
    if (_versionHashes.length != _validities.length) {
        revert AssetPluginRegistry__LengthMismatch();
    }
    if (_asset == address(0)) {
        revert AssetPluginRegistry__InvalidAsset();
    }
    if (isDeprecated[_asset]) {
        revert AssetPluginRegistry__DeprecatedAsset();
    }

    // ... (rest of the function remains the same)
}

This fix ensures that deprecated assets cannot be updated, maintaining consistency with the isValidAsset function and the overall deprecation mechanism of the contract.

Additionally, you should add a new custom error:

error AssetPluginRegistry__DeprecatedAsset();

Assessed type

Invalid Validation