Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Missing `__gap` variable declaration in the `OperatorVCS` contract which can cause some storage collision during update.

0xrolko

Relevant GitHub Links

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorVCS.sol#L11

Summary

Risk of storage collision during update.

Vulnerability Details

The OperatorVCS contract inherit from the VaultControllerStrategy

contract OperatorVCS inherits contract VaultControllerStrategy, which in turn inherits contract Strategy. The latter inherits UUPSUpgradeable, which means that all its children can be upgraded. And when it comes to upgrading smart contracts, one of the most important points to bear in mind is the collision of storage spaces. For this reason, we recommend adding a gap variable to reserve space for future changes. However, in this style of inheritance, this reservation should only be made in contracts that introduce new variables in addition to those they inherit. And in our situation `OperatorVCS` introduces new variables but no space has yet been reserved for future changes, which can cause a storage collision during the update:

contract OperatorVCS is VaultControllerStrategy {
using SafeERC20Upgradeable for IERC20Upgradeable;
​
// basis point amount of an operator's earned rewards that they receive
uint256 public operatorRewardPercentage;
// total unclaimed operator LST rewards
uint256 private unclaimedOperatorRewards;
​
// used to check vault membership in this strategy
mapping(address => bool) private vaultMapping;
// list of vaults that are queued for removal
address[] private vaultsToRemove;
​
@> /** @audit should add new __gap here to reserve storage since this child contract introduce his own storage variable */
​
event VaultAdded(address indexed operator);
​
/// ... The rest of code
}

Impact

Storage conflicts during the update.

Tools Used

Manual analysis.

Recommendations

contract OperatorVCS is VaultControllerStrategy {
using SafeERC20Upgradeable for IERC20Upgradeable;
​
// basis point amount of an operator's earned rewards that they receive
uint256 public operatorRewardPercentage;
// total unclaimed operator LST rewards
uint256 private unclaimedOperatorRewards;
​
// used to check vault membership in this strategy
mapping(address => bool) private vaultMapping;
// list of vaults that are queued for removal
address[] private vaultsToRemove;
​
+ uint256[10] private __gap;;
​
event VaultAdded(address indexed operator);
​
/// ... The rest of code
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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