Potential Risk that Deprecated Assets can be Used as Valid Ones

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L148-L154 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L159-L167 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L126-L129 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L134-L138

Vulnerability details

Impact

Deprecated assets by AssetPluginRegistry can be retrieved as normal ones in backing manager and revenue traders via the AssetRegistry::getRegistry() and AssetRegistry::erc20s() functions, and may cause a potential danger to the protocol.

Proof of Concept

AssetPluginRegistry contract provides additional asset management functionality including deprecation.

But the deprecation of an asset is checked only when it is being registered via the _registerIgnoringCollisions() function, and once regstered, deprecation is not considered when calling asset registry functions, so deprecated assets can be provided as valid ones to the backing manager, revenue trader, etc.

And more dangerously, deprecated assets can be parts of prime baskets when a governer tries or force to set them.

Tools Used

Manual Review

Recommended Mitigation Steps

I suggest updating AssetRegistry::getRegistry() function by adding validation checks like the below:

    function getRegistry() public view returns (Registry memory reg) {
        uint256 length = _erc20s.length();
        reg.erc20s = new IERC20[](length);
        reg.assets = new IAsset[](length);
        for (uint256 i = 0; i < length; ++i) {
            if (address(assetPluginRegistry) != address(0)) {
                if (assetPluginRegistry.isValidAsset(
                        keccak256(abi.encodePacked(this.version())),
                        address(assets[IERC20(_erc20s.at(i))])
                    )) {
                    reg.erc20s[i] = IERC20(_erc20s.at(i));
                    reg.assets[i] = assets[IERC20(_erc20s.at(i))];
                }
            }
        }
    }

And also do this to the erc20s(), toAsset(), toColl() functions.

Assessed type

Invalid Validation