Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: low
Valid

Upgrading `OperatorVCS` Contract Will Fail

Summary

The upgrade process for the OperatorVCS contract will fail due to a versioning conflict in the initialize(...) function. This occurs because of passing the same version number as the previous contract in the reinitializer(...) modifier. As a result, the new state variables introduced in the upgraded contract will not be initialized, leading to a failure in contract functionality.

Vulnerability Details

The OperatorVCS contract being audited is an upgraded version of an already deployed contract (proxy address, implementation deployment). This new version introduces additional state variables, such as _vaultMaxDeposits and _vaultDepositController, which need to be initialized.

However, in the new version of the contract, the initialize(...) function is decorated with the reinitializer(3) modifier from OpenZeppelin. This modifier restricts the reinitialization process by allowing the function to be called only once, and only if the contract has not already been initialized to a version greater than 3.

The issue arises because the previously deployed version of the contract also used reinitializer(3). As a result, when attempting to upgrade, the modifier detects that version 3 has already been initialized, blocking the execution of the initialize(...) function in the new version. This effectively prevents the initialization of new state variables introduced in the upgraded contract.
Audited Version of OperatorVCS Contract:

// @audit the audited version of OperatorVCS contract
function initialize(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits, // new state variable
uint256 _operatorRewardPercentage,
address _vaultDepositController // new state variable
) public reinitializer(3) { // it was 3 in the previous version, this one should be 4
if (address(token) == address(0)) {
__VaultControllerStrategy_init(
_token,
_stakingPool,
_stakeController,
_vaultImplementation,
_fees,
_maxDepositSizeBP,
_vaultMaxDeposits,
_vaultDepositController
);
}
// ...
}

Deployed Version of OperatorVCS Contract:

// The deployed version of OperatorVCS contract
function initialize(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _operatorRewardPercentage
) public reinitializer(3) {
if (address(token) == address(0)) {
__VaultControllerStrategy_init(
_token,
_stakingPool,
_stakeController,
_vaultImplementation,
_fees,
_maxDepositSizeBP
);
}
// ...
}

Impact

The failure to upgrade the contract means that the new state variables introduced in the upgraded version (_vaultMaxDeposits and _vaultDepositController) will not be initialized, resulting in a critical failure for the new contract features. This will prevent the proper functioning of any logic that relies on these variables. The contract will revert whenever the initialize(...) function is called.

Tools Used

  • Manual review

POC

// add this file to /test folder
// update hardhat.config.ts to include the following:
/*localhost: {
url: 'http://127.0.0.1:8545',
forking: {
url: "https://eth-mainnet.g.alchemy.com/v2/<key>",
blockNumber: 19185204
}
},*/
// run `npx hardhat node --fork https://eth-mainnet.g.alchemy.com/v2/<key> --fork-block-number 19185204`
// npx hardhat test test/upgrade.attack.test.ts --network localhost
import hre,{ ethers ,upgrades} from 'hardhat'
import{ impersonateAccount } from "@nomicfoundation/hardhat-toolbox/network-helpers";
import { expect } from 'chai';
describe('StakingPool', () => {
it('Should fail when upgrading OperatorVCS because of the sed contract version ', async () => {
const ownerOrDeployer = "0xB351EC0FEaF4B99FdFD36b484d9EC90D0422493D"; // https://etherscan.io/address/0xB351EC0FEaF4B99FdFD36b484d9EC90D0422493D
const proxyAddress= "0x4852e48215A4785eE99B640CACED5378Cc39D2A4"; // https://etherscan.io/address/0x4852e48215A4785eE99B640CACED5378Cc39D2A4#code
const implementationContractName = "OperatorVCS";
await impersonateAccount(ownerOrDeployer);
const impersonatedSigner = await ethers.getSigner(ownerOrDeployer);
console.log("impersonatedSigner",impersonatedSigner.address);
await fundAccount(ownerOrDeployer);
// await startPrank(ownerOrDeployer);
const Contract = (await ethers.getContractFactory(implementationContractName)).connect(impersonatedSigner);
await expect( upgrades.upgradeProxy(proxyAddress, Contract, { call:{fn:'initialize', args:[
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
ethers.ZeroAddress,
[],
0,
0,0, ethers.ZeroAddress]},
kind: 'uups',
unsafeAllow: ['delegatecall'],
unsafeSkipStorageCheck: true,
})).to.be.rejectedWith("Initializable: contract is already initialized");
// await contract.waitForDeployment()
// await stopPrank(ownerOrDeployer);
})
})
const stopPrank = async (wallet:string) => {
await hre.network.provider.request({
method: "hardhat_stopImpersonatingAccount",
params: [wallet],
});
}
const startPrank = async (wallet:string) => {
await hre.network.provider.request({
method: "hardhat_impersonateAccount",
params: [wallet],
});
}
const fundAccount = async (wallet: string) => {
await hre.network.provider.send("hardhat_setBalance", [
wallet,
"0x" + ethers.parseEther("100").toString(16),
]);
};

Recommendations

To resolve the issue, the version number in the reinitializer(...) modifier should be incremented from 3 to 4 in the new version of the contract. This will allow the initialize(...) function to execute successfully during the upgrade process and initialize the new state variables.

reinitializer(4) // Update to the next version number
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Some contracts will not be initialized due to an incorrect `reinitializer` versions used

Support

FAQs

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