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

Missing storage gap in upgradeability `StabilityPool` contract

r4bbit

Summary

The StabilityPool aims to be upgradeable but is prone to storage collisions when not upgraded carefully. Adding a storage gap fixes this.

Vulnerability Details

The StabilityPool aims to be upgradeable. When dealing with upgradeable contracts there's always a risk of storage collisions. This is because changing or adding a variable in a contract that's upgraded, can change the storage layout, which conflicts with the currently deployed version, resulting in corrupted data and the contract possibly no longer working.

Therefore, upgradeable contracts ideally make use of storage gaps to "reserve" a slot space for future upgrades.

Impact

In future upgrades of StabilityPool, the contract could introduce storage collisions when either changing or adding variables, or when it's becoming part of an inheritance chain (which is often the recommended way to extend contracts as part of an upgrade).

Tools Used

Manual review.

Recommendations

Introduce a storage gap to reserve some space in the contract's layout. This way adding fields to the upgradeable contract is possible without introducing collisions of storage slots.

Here's what this could look like:

contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuard, OwnableUpgradeable, PausableUpgradeable {
using SafeERC20 for IERC20;
using SafeERC20 for IRToken;
using SafeERC20 for IDEToken;
using SafeERC20 for IRAACToken;
​
// State variables
IRToken public rToken;
IDEToken public deToken;
IRAACToken public raacToken;
ILendingPool public lendingPool;
IERC20 public crvUSDToken;
​
// Manager variables (manger can liquidate as stability pool)
mapping(address => bool) public managers;
// Manager value allocation / allowance
mapping(address => uint256) public managerAllocation;
uint256 public totalAllocation;
address[] public managerList;
​
mapping(address => uint256) public userDeposits;
​
IRAACMinter public raacMinter;
address public liquidityPool;
​
mapping(address => bool) public supportedMarkets;
mapping(address => uint256) public marketAllocations;
​
uint256 public lastUpdate;
uint256 public index = 1e18;
​
address private immutable _initialOwner;
​
// Allow to make rToken / deToken decimals flexible
uint8 public rTokenDecimals;
uint8 public deTokenDecimals;
​
+. uint256[50] __gap;
​
...
}

Relevant links

Similar findings

  • https://solodit.cyfrin.io/issues/m-07-smartaccountsol-is-intended-to-be-upgradable-but-inherits-from-contracts-that-contain-storage-and-no-gaps-code4rena-biconomy-biconomy-smart-contract-wallet-contest-git

  • https://solodit.cyfrin.io/issues/m-18-future-upgrades-may-be-difficult-or-impossible-sherlock-elfi-git

  • https://solodit.cyfrin.io/issues/m-05-no-storage-gap-for-upgradeable-contract-might-lead-to-storage-slot-collision-code4rena-alchemix-alchemix-contest-git

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!