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

Use of Non‑Upgradeable ReentrancyGuard in an Upgradeable Contract

cipherhawk

Overview

The StabilityPool contract is built as an upgradeable contract using OpenZeppelin’s OwnableUpgradeable and PausableUpgradeable. However, it imports and uses the standard (non‑upgradeable) ReentrancyGuard from OpenZeppelin. Upgradeable contracts must carefully manage their storage layout. The non‑upgradeable ReentrancyGuard does not use an initializer and may occupy storage slots in a way that conflicts with upgradeable patterns. This can lead to unexpected behavior in reentrancy protection or even allow an attacker to exploit storage collisions if the upgrade process is not managed correctly.

Root Cause & Potential Impact

  • Storage Layout Conflicts:
    The non‑upgradeable ReentrancyGuard sets up its internal state (e.g. a private _status variable) in the constructor. In an upgradeable contract (which relies on initializer functions rather than constructors), the storage layout is defined by the order of inherited state variables. Mixing upgradeable base contracts with a non‑upgradeable ReentrancyGuard can lead to overlapping storage slots or uninitialized guard variables.

  • Exploitation Path:
    Although no immediate attack vector is evident (the contract’s functions are marked nonReentrant and appear to work in tests), the risk is that if an upgrade is performed or if an attacker can force reinitialization, the reentrancy guard might not properly protect critical functions. This could open the door for reentrancy attacks on deposit, withdraw, or liquidation flows, potentially leading to loss of funds.

  • Maintenance Risk:
    Future upgrades or changes in the storage layout may inadvertently break the intended reentrancy protection, causing subtle bugs that could be exploited in production.

Foundry PoC / Illustrative Test

Demonstrating a storage layout collision in a Foundry test can be challenging without a full proxy setup. However, we can illustrate the principle via a simplified test that shows how storage variables from non‑upgradeable contracts might be mis‑initialized when used with upgradeable contracts.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "@openzeppelin/contracts/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
​
// A simplified non-upgradeable ReentrancyGuard
contract NonUpgradeableReentrancyGuard {
uint256 private _status;
​
constructor() {
_status = 1; // Not entered.
}
​
modifier nonReentrant() {
require(_status == 1, "Reentrant call");
_status = 2;
_;
_status = 1;
}
}
​
// An upgradeable contract that inherits from OwnableUpgradeable and our non-upgradeable guard.
contract TestUpgradeable is Initializable, OwnableUpgradeable, NonUpgradeableReentrancyGuard {
uint256 public counter;
​
function initialize() public initializer {
__Ownable_init();
counter = 0;
}
​
function increment() external nonReentrant {
counter += 1;
}
}
​
contract ReentrancyGuardUpgradeableTest is Test {
TestUpgradeable testContract;
​
function setUp() public {
testContract = new TestUpgradeable();
testContract.initialize();
}
​
function testIncrementWorks() public {
testContract.increment();
assertEq(testContract.counter(), 1);
}
}
​

-->Note:
This test demonstrates that the contract works “as is” in a controlled environment. However, because the non‑upgradeable guard’s storage layout isn’t designed for upgradeable contracts, its use poses a maintenance risk. In a production upgradeable system, the correct approach is to use OpenZeppelin’s ReentrancyGuardUpgradeable.

Mitigation

Recommended Fix:
Replace the non‑upgradeable ReentrancyGuard with ReentrancyGuardUpgradeable. For example:

import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
​
contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuardUpgradeable, OwnableUpgradeable, PausableUpgradeable {
// ... rest of code ...
​
function initialize(
address _rToken,
address _deToken,
address _raacToken,
address _raacMinter,
address _crvUSDToken,
address _lendingPool
) public initializer {
__Ownable_init(_initialOwner);
__Pausable_init();
__ReentrancyGuard_init();
// ... rest of initialization ...
}
}
​

Using the upgradeable version ensures that storage layout is consistent with the rest of the upgradeable contracts and that the reentrancy guard is properly initialized.

Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!