Silent Code Execution When Deposit is above Deposit Cap

Summary

Silent Code Execution When Deposit is above Deposit Cap in the RioLRTCoordinator.sol contract

Vulnerability Detail

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTAssetRegistry.sol#L270

    /// @dev Sets the asset's deposit cap.
    /// @param newDepositCap The new rebalance delay.
    function setAssetDepositCap(address asset, uint96 newDepositCap) external onlyOwner {
        if (!isSupportedAsset(asset)) revert ASSET_NOT_SUPPORTED(asset);

>>>        assetInfo[asset].depositCap = newDepositCap;

        emit AssetDepositCapSet(asset, newDepositCap);
    }

The function above from the RioLRTAssetRegistry.sol contract shows how Owner sets new DepositCap, In fact DepositCap can be set to any value, zero including. However a look at the _checkDepositCapReached(..) function below from the RioLRTCoordinator.sol contract shows that this expected implementatation is rendered useless whenever DepositCap is Zero, The correct implementation would be to revert the _checkDepositCapReached in this circumstances however the Code would silently be executed without reversion

    function _checkDepositCapReached(address asset, uint256 amountIn) internal view {
        IRioLRTAssetRegistry assetRegistry_ = assetRegistry();

>>        uint256 depositCap = assetRegistry_.getAssetDepositCap(asset);
>>>        if (depositCap > 0) {
            uint256 existingBalance = assetRegistry_.getTotalBalanceForAsset(asset);
>>>            if (existingBalance + amountIn > depositCap) {
                revert DEPOSIT_CAP_REACHED(asset, depositCap);
            }
        }
    }

Impact

Silent Code Execution When Deposit is above Deposit Cap in the RioLRTCoordinator.sol contract

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTCoordinator.sol#L288 https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTAssetRegistry.sol#L270

Tool used

Manual Review

Recommendation

Deposit AmountIn should always be checked if it is above Deposit Cap even when DepositCap is Zero to ensure the code actually reverts instead of just silently executing the function calling it. And the Protocol should also consider not allowing Owner to set DepositCap to Zero at all in the first place.

    function _checkDepositCapReached(address asset, uint256 amountIn) internal view {
        IRioLRTAssetRegistry assetRegistry_ = assetRegistry();

        uint256 depositCap = assetRegistry_.getAssetDepositCap(asset);
---        if (depositCap > 0) {
            uint256 existingBalance = assetRegistry_.getTotalBalanceForAsset(asset);
           if (existingBalance + amountIn > depositCap) {
                revert DEPOSIT_CAP_REACHED(asset, depositCap);
            }
---        }
    }

sherlock-audit / 2024-02-rio-network-core-protocol-judging

Topmark - Silent Code Execution When Deposit is above Deposit Cap #39