search

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

4 stars 4 forks source link

hunter_w3b - There is `off-by-one` error occurring in `RioLRTAssetRegistry::_addAsset` #336

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

hunter_w3b

medium

There is off-by-one error occurring in RioLRTAssetRegistry::_addAsset

Summary

Due to an off-by-one error in the RioLRTAssetRegistry::_addAsset function, ERC20 tokens with 18 decimals are incorrectly reverted and not added to the supportedAssets array, as the condition if (decimals > 18) improperly rejects them alongside tokens with more than 18 decimals, exacerbated by similar initialization of priceFeedDecimals to 18 in the RioLRTAssetRegistry::initialize function.

Vulnerability Detail

In the RioLRTAssetRegistry::initialize function, we set the value of the priceFeedDecimals variable to either 8 or 18 and revert the transaction if this condition is true when the value of priceFeedDecimals is 18.

In the RioLRTAssetRegistry::_addAsset function, if the config.asset == ETH_ADDRESS, decimals are stored as 18, then this if statement is executed.

        if (config.asset == ETH_ADDRESS) {
            if (config.priceFeed != address(0)) revert INVALID_PRICE_FEED();
            if (config.strategy != BEACON_CHAIN_STRATEGY) revert INVALID_STRATEGY();
        }

However, when config.asset is not ETH_ADDRESS, we assign the value of IERC20Metadata(config.asset).decimals() to the uint8 decimals. Since the protocol supports ERC20 tokens with 18 decimals and also initialize priceFeedDecimals with 18 decimals, if the function encounters a token with more than 18 decimals, it must revert. However, when the ERC20 token has 18 decimals, it also reverts and does not add it to the supportedAssets array.

Because of this off-by-one error:

            if (decimals > 18) revert INVALID_ASSET_DECIMALS();

Impact

ERC20 tokens that have 18 decimals do not get added to the supportedAssets array.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

The decimal value must be equal to 18 or smaller than 18.

-            if (decimals > 18) revert INVALID_ASSET_DECIMALS();
+            if (decimals >= 18) revert INVALID_ASSET_DECIMALS();
nevillehuang commented 7 months ago

Invalid, check is correct