Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Use of incorrect `ReentrancyGuard` in `StabilityPool`

r4bbit

Summary

Incorrect ReentrancyGuard version results in reentrancy guard not being initialized correctly, which can potentially break reentrancy protection.

Vulnerability Details

The StabilityPool contract in the RAAC protocol aims to be upgradeable. To ensure this, it makes use of some upgradeable variations of Open Zeppelin's contracts, such as OwnableUpgradeable and PausableUpgradeable.

contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuard, OwnableUpgradeable, PausableUpgradeable {
...
}

This is crucial because constructors are not executed in upgradeable contracts. That's why there's a dedicated initialize function that has to be called by the proxy that calls into the implementation contract. It takes care of initializing parent contracts as well.

function initialize(
address _rToken,
address _deToken,
address _raacToken,
address _raacMinter,
address _crvUSDToken,
address _lendingPool
) public initializer {
if (_rToken == address(0) || _deToken == address(0) || _raacToken == address(0) || _raacMinter == address(0) || _crvUSDToken == address(0) || _lendingPool == address(0)) revert InvalidAddress();
__Ownable_init(_initialOwner); // <--
__Pausable_init(); // <--
...
}

The ReentrancyGuard however, is imported as the non-upgradeable version. Which means it's not properly initialized and could even cause storage collisions in future upgrades. It's nonReentrant modifier is used on crucial functions of the contract, so it's very important that is set up correctly.

Impact

Using the non-upgradeable version causes the ReentrancyGuard to not be initialized correctly, which could potentially break reentrancy protection.

Tools Used

Manual review.

Recommendations

Import, inherit and initialize the ReentrancyGuardUpgradeable version instead as shown below:

import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
+ import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
​
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
- import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
...
​
- contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuard, OwnableUpgradeable, PausableUpgradeable {
+ contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuardUpgradeable, OwnableUpgradeable, PausableUpgradeable {
...
function initialize(
address _rToken,
address _deToken,
address _raacToken,
address _raacMinter,
address _crvUSDToken,
address _lendingPool
) public initializer {
if (_rToken == address(0) || _deToken == address(0) || _raacToken == address(0) || _raacMinter == address(0) || _crvUSDToken == address(0) || _.lendingPool == address(0)) revert InvalidAddress();
__Ownable_init(_initialOwner);
__Pausable_init();
+. __ReentrancyGuard_init();
...
}
...
}

Relevant links

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.