Topmark - Asset with Invalid Token Decimal can be Added to Rio Asset Registry

[sherlock-admin4]

Topmark

high

Asset with Invalid Token Decimal can be Added to Rio Asset Registry

Summary

Asset with Invalid Token Decimal can be Added to Rio Asset Registry when addAsset(...) function is called in the RioLRTAssetRegistry.sol contract and priceFeedDecimals is wrongly validated during Initialization

Vulnerability Detail

function _addAsset(AssetConfig calldata config) internal {
        if (isSupportedAsset(config.asset)) revert ASSET_ALREADY_SUPPORTED(config.asset);
        if (config.asset == address(0)) revert INVALID_ASSET_ADDRESS();

        uint8 decimals = config.asset == ETH_ADDRESS ? 18 : IERC20Metadata(config.asset).decimals();
        if (config.asset == ETH_ADDRESS) {
            if (config.priceFeed != address(0)) revert INVALID_PRICE_FEED();
            if (config.strategy != BEACON_CHAIN_STRATEGY) revert INVALID_STRATEGY();
        } else {
>>>            if (decimals > 18) revert INVALID_ASSET_DECIMALS();
            if (config.priceFeed == address(0)) revert INVALID_PRICE_FEED();
            if (IPriceFeed(config.priceFeed).decimals() != priceFeedDecimals) revert INVALID_PRICE_FEED_DECIMALS();
            if (IStrategy(config.strategy).underlyingToken() != config.asset) revert INVALID_STRATEGY();
        }
        supportedAssets.push(config.asset);

        AssetInfo storage info = assetInfo[config.asset];
        info.decimals = decimals;
        info.depositCap = config.depositCap;
        info.priceFeed = config.priceFeed;
        info.strategy = config.strategy;

        emit AssetAdded(config);
    }

The function above shows how _addAsset(...) is being added to the RioLRTAssetRegistry.sol contract, the problem with the implementation is that the decimal only stops decimal values that are above 18 as noted from the pointer. but a look at the contract shows that only token with 8 and 18 decimal is allowed depending on if it is Non-Eth and Eth respectively, therefore validation should specifically target 8 and 18 and not just simply stopping above 18 tokens. A look at L58 of the same contract gives an idea of how the decimal values should have been implemented. But it can be noted from the pointer in the code below that Non-ETH pairs must use 8 decimals, while ETH pairs must use 18 but the implementation made a mistake by handling both validations together instead of separately, meaning a Non-Eth pairs that is 18 not 8 would also pass through when only Eth of 18 is allowed

   function initialize(
        address initialOwner,
        address token_,
        uint8 priceFeedDecimals_,
        AssetConfig[] calldata initialAssets
    ) external initializer {
        __Ownable_init(initialOwner);
        __UUPSUpgradeable_init();
        __RioLRTCore_init(token_);

>>        // Non-ETH pairs must use 8 decimals, while ETH pairs must use 18.
        if (priceFeedDecimals_ != 8 && priceFeedDecimals_ != 18) revert INVALID_PRICE_FEED_DECIMALS();
//@audit Non-Eth pairs of 18 would pass through instead of just 8
        priceFeedDecimals = priceFeedDecimals_;
 ....

Impact

Asset with Invalid Token Decimal can be Added to Rio Asset Registry when addAsset(...) function is called in the RioLRTAssetRegistry.sol contract and priceFeedDecimals is wrongly validated during Initialization which would break protocol

Code Snippet

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

Tool used

Manual Review

Recommendation

Protocol should make necessary adjustment to L58 to ensure priceFeedDecimals is 8 uniquely for Non-ETH pairs and priceFeedDecimals is 18 specifically for ETH pairs. Then The Decimal validation at _addAsset(...) function should be adjusted accordingly as provided below to prevent unwanted tokens.

function _addAsset(AssetConfig calldata config) internal {
        if (isSupportedAsset(config.asset)) revert ASSET_ALREADY_SUPPORTED(config.asset);
        if (config.asset == address(0)) revert INVALID_ASSET_ADDRESS();

        uint8 decimals = config.asset == ETH_ADDRESS ? 18 : IERC20Metadata(config.asset).decimals();
        if (config.asset == ETH_ADDRESS) {
            if (config.priceFeed != address(0)) revert INVALID_PRICE_FEED();
            if (config.strategy != BEACON_CHAIN_STRATEGY) revert INVALID_STRATEGY();
        } else {
---            if (decimals > 18) revert INVALID_ASSET_DECIMALS();
+++          if (decimals != 8 && decimals != 18) revert INVALID_ASSET_DECIMALS();
            if (config.priceFeed == address(0)) revert INVALID_PRICE_FEED();
            if (IPriceFeed(config.priceFeed).decimals() != priceFeedDecimals) revert INVALID_PRICE_FEED_DECIMALS();
            if (IStrategy(config.strategy).underlyingToken() != config.asset) revert INVALID_STRATEGY();
        }
        supportedAssets.push(config.asset);

        AssetInfo storage info = assetInfo[config.asset];
        info.decimals = decimals;
        info.depositCap = config.depositCap;
        info.priceFeed = config.priceFeed;
        info.strategy = config.strategy;

        emit AssetAdded(config);
    }

Duplicate of #168

[solimander]

Only price feed decimals MUST be 8 or 18, not asset decimals.