Summary
Missing Storage Gap in Upgradable Contract
Vulnerability Details
The contracts OperatorVCS and CommunityVCS inherit from the following base contracts:
OperatorVCS -> VaultControllerStrategy -> Strategy
CommunityVCS -> VaultControllerStrategy -> Strategy
As demonstrated in the code snippets below, both OperatorVCS and CommunityVCS lack the necessary storage gaps required for upgradeability:
abstract contract Strategy is IStrategy, Initializable, UUPSUpgradeable, OwnableUpgradeable {
IERC20Upgradeable public token;
IStakingPool public stakingPool;
}
abstract contract VaultControllerStrategy is Strategy {
uint256[4] private __gap;
}
contract OperatorVCS is VaultControllerStrategy {
uint256 public operatorRewardPercentage;
uint256 private unclaimedOperatorRewards;
}
contract CommunityVCS is VaultControllerStrategy {
uint128 public vaultDeploymentThreshold;
uint128 public vaultDeploymentAmount;
}
The same issue applies to the OperatorVault and CommunityVault contracts:
OperatorVault -> Vault
CommunityVault -> Vault
Both contracts also lack the necessary storage gaps:
abstract contract Vault is Initializable, UUPSUpgradeable, OwnableUpgradeable {
uint256[9] private __gap;
}
contract OperatorVault is Vault {
address public operator;
address public rewardsReceiver;
IPFAlertsController public pfAlertsController;
uint128 public trackedTotalDeposits;
uint128 private unclaimedRewards;
}
contract CommunityVault is Vault {
}
This issue extends to other critical contracts like OperatorStakingPool, FundFlowController, WithdrawalPool, and more, all of which lack storage gaps:
OperatorStakingPool
contract OperatorStakingPool is Initializable, UUPSUpgradeable, OwnableUpgradeable {
IStakingPool public lst;
address[] private operators;
mapping(address => bool) private operatorMap;
mapping(address => uint256) private shareBalances;
uint256 private totalShares;
uint256 public depositLimit;
}
FundFlowController
contract FundFlowController is UUPSUpgradeable, OwnableUpgradeable {
IVaultControllerStrategy public operatorVCS;
IVaultControllerStrategy public communityVCS;
uint64 public unbondingPeriod;
uint64 public claimPeriod;
uint64 public numVaultGroups;
uint64 public curUnbondedVaultGroup;
uint256[] public timeOfLastUpdateByGroup;
}
WithdrawalPool
contract WithdrawalPool is UUPSUpgradeable, OwnableUpgradeable {
IERC20Upgradeable public token;
IERC20Upgradeable public lst;
IPriorityPool public priorityPool;
Withdrawal[] internal queuedWithdrawals;
mapping(address => uint256[]) internal queuedWithdrawalsByAccount;
mapping(uint256 => address) internal withdrawalOwners;
uint256 internal totalQueuedShareWithdrawals;
uint256 public indexOfNextWithdrawal;
WithdrawalBatch[] internal withdrawalBatches;
uint128 public withdrawalBatchIdCutoff;
uint128 public withdrawalIdCutoff;
uint256 public minWithdrawalAmount;
uint64 public minTimeBetweenWithdrawals;
uint64 public timeOfLastWithdrawal;
}
StakingPool -> StakingRewardsPool
abstract contract StakingRewardsPool is ERC677Upgradeable, UUPSUpgradeable, OwnableUpgradeable {
}
contract StakingPool is StakingRewardsPool {
address[] private strategies;
uint256 public totalStaked;
uint256 public unusedDepositLimit;
Fee[] private fees;
address public priorityPool;
address public rebaseController;
uint16 private poolIndex;
}
PriorityPool
contract PriorityPool is UUPSUpgradeable, OwnableUpgradeable, PausableUpgradeable {
IERC20Upgradeable public token;
IStakingPool public stakingPool;
ISDLPool public sdlPool;
address public distributionOracle;
uint128 public queueDepositMin;
uint128 public queueDepositMax;
PoolStatus public poolStatus;
bytes32 public merkleRoot;
bytes32 public ipfsHash;
uint256 public merkleTreeSize;
uint256 public totalQueued;
uint256 public depositsSinceLastUpdate;
uint256 private sharesSinceLastUpdate;
address[] private accounts;
mapping(address => uint256) private accountIndexes;
mapping(address => uint256) private accountQueuedTokens;
mapping(address => uint256) private accountClaimed;
mapping(address => uint256) private accountSharesClaimed;
address public rebaseController;
IWithdrawalPool public withdrawalPool;
}
Upgradeable contracts must preserve their storage layout across contract versions because the proxy retains the storage structure of the original implementation. Without a storage gap, adding new state variables during an upgrade can overwrite existing variables in derived contracts, leading to undefined behavior or security risks.
The absence of storage gaps violates the best practices for maintaining future-proof upgradeable contracts. A well-defined storage gap reserves extra storage slots to ensure that future variables can be added without interfering with the existing layout.
Impact
Failing to reserve storage slots for future upgrades risks overwriting existing storage variables during contract upgrades, potentially causing unpredictable behavior or introducing security vulnerabilities.
Tools Used
Manual Review
Recommendations
It is recommended to add appropriate storage gaps in the upgradeable contracts to reserve space for future variables. For instance, the following code snippet can be added to the contracts:
uint256[10] private __gap;
This will ensure compatibility with future upgrades while maintaining the integrity of the contract's storage layout.
