Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L69-L75 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/AssetRegistry.sol#L142-L144 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L70

Vulnerability details

grantRTokenAllowance function in the BackingManager contract is reverting for registered ERC20 tokens due to an incorrect isRegistered check in the AssetRegistry contract. This will cause the RToken contract to unexpectedly fail to receive allowances for registered tokens, impacting the system's ability to manage and trade these assets as the BackingManager will be unable to grant the necessary allowances.

Root Cause

In the BackingManager contract, the grantRTokenAllowance function is responsible for granting the RToken contract allowance to spend registered ERC20 tokens held by the BackingManager. However, there is an issue with the isRegistered check performed on the input erc20 token address: BackingManager.sol#L64-L74

function grantRTokenAllowance(IERC20 erc20) external notFrozen {
    require(assetRegistry.isRegistered(erc20), "erc20 unregistered");
    // ...
}

The isRegistered function in AssetRegistry contract is returning false for registered tokens, causing the require statement in grantRTokenAllowance to revert unexpectedly.

Impact

The RToken contract suffers from an inability to receive allowances for registered ERC20 tokens. This prevents the RToken from being able to manage and trade these assets as expected, potentially leading to system instability and loss of functionality.

Proof of Concept

Pre-conditions

The AssetRegistry contract is initialized with a set of registered ERC20 tokens.
The BackingManager contract is initialized and linked to the AssetRegistry.

Steps

Call grantRTokenAllowance(registeredERC20) on the BackingManager contract, passing a registered ERC20 token address.
The require(assetRegistry.isRegistered(erc20), "erc20 unregistered") statement in BackingManager.sol:70 unexpectedly reverts.
The RToken contract fails to receive the necessary allowance for the registered token.

Tools Used

Manual code review

Recommended Mitigation Steps

The isRegistered function in the AssetRegistry contract should correctly return true for registered ERC20 tokens.

// AssetRegistry.sol
function isRegistered(IERC20 erc20) external view returns (bool) {
-   return _erc20s.contains(address(erc20));
+   return assets[erc20] != IAsset(address(0));
}

Assessed type

Other

code-423n4 / 2024-07-reserve-validation

Registered ERC20 Tokens Unexpectedly Fail Allowance Granting #128