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

Upgrade Initialization Logic Will Never Execute Due to Incorrect Initializer Usage in CommunityVCS

tinnohofficial

Summary

The CommunityVCS contract contains upgrade initialization logic within its initialize function that's guarded by the OpenZeppelin initializer modifier. Due to how this modifier works, the upgrade logic will never execute as the modifier will revert during upgrades, leaving the contract in a potentially inconsistent state.

Vulnerability Details

The initialize function in CommunityVCS contains two paths: one for first-time initialization and another for upgrades, determined by checking if token address is zero. However, the entire function is guarded by OpenZeppelin's initializer modifier which prevents any subsequent calls after the first initialization. This means the upgrade path in the else block becomes unreachable code as the modifier will revert before reaching it.

function initialize(...) public initializer {
if (address(token) == address(0)) {
// First initialization logic ...
} else {
// @audit Upgrade initialization - UNREACHABLE!
globalVaultState = GlobalVaultState(5, 0, 0, uint64(maxDepositSizeBP + 1));
maxDepositSizeBP = _maxDepositSizeBP;
delete fundFlowController;
vaultMaxDeposits = _vaultMaxDeposits;
}
// ...
}

as it can be seen below OpenZeppelin's initializer modifier ensures the function can only be called when _initialized < 1. And since _initialized is set to 1 on the proxy when the very first CommunityVCS version is initialized, it means that any other call to the initialize function would revert essentially making the else block in the function above unreachable.

modifier initializer() {
bool isTopLevelCall = !_initializing;
require(
@> (isTopLevelCall && _initialized < 1) || (!AddressUpgradeable.isContract(address(this)) && _initialized == 1),
"Initializable: contract is already initialized"
);
_initialized = 1; // @audit this will be set on the proxy when the very first version is initialized
if (isTopLevelCall) {
_initializing = true;
}
_;
if (isTopLevelCall) {
_initializing = false;
emit Initialized(1);
}
}

PoC

  1. Deploy CommunityVCS contract and call initialize() - succeeds

  2. Upgrade the contract implementation

  3. Try to call initialize() again - reverts with "Initializable: contract is already initialized"

  4. upgrade initialization logic in the else block never executes

Impact

The upgrade initialization logic that sets critical contract parameters (globalVaultState, maxDepositSizeBP, fundFlowController, vaultMaxDeposits) will never execute during upgrades. This could leave the contract in an inconsistent state with outdated or incorrect parameters, potentially affecting core functionality like deposit limits and vault management.

Tools Used

Manual review

Recommendations

Separate the initialization and upgrade logic into two distinct functions using OpenZeppelin's reinitializer pattern.
Note: only add the reinitialize function in the implementation to be upgraded to

- function initialize(...) public initializer {
- if (address(token) == address(0)) {
- // First initialization
- ...
- } else {
- // Upgrade initialization
- ...
- }
- }
​
+ function initialize(...) public initializer {
+ // First initialization only
+ __VaultControllerStrategy_init(...);
+ vaultDeploymentThreshold = _vaultDeploymentThreshold;
+ vaultDeploymentAmount = _vaultDeploymentAmount;
+ _deployVaults(_vaultDeploymentAmount);
+ globalVaultState = GlobalVaultState(5, 0, 0, 0);
+ }
+
+ // Add this function only when you want to do the upgrade
+ function reinitialize(...) public reinitializer(version) { // version could be 2, 3, etc
+ // Upgrade initialization
+ globalVaultState = GlobalVaultState(5, 0, 0, uint64(maxDepositSizeBP + 1));
+ maxDepositSizeBP = _maxDepositSizeBP;
+ delete fundFlowController;
+ vaultMaxDeposits = _vaultMaxDeposits;
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Upgrade Initialization Logic Will Never Execute Due to Incorrect Initializer Usage in CommunityVCS

Support

FAQs

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