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:
function initialize(
address _token,
address _stakingPool,
address _stakeController,
address _vaultImplementation,
Fee[] memory _fees,
uint256 _maxDepositSizeBP,
uint256 _vaultMaxDeposits,
uint256 _operatorRewardPercentage,
address _vaultDepositController
) public reinitializer(3) {
if (address(token) == address(0)) {
__VaultControllerStrategy_init(
_token,
_stakingPool,
_stakeController,
_vaultImplementation,
_fees,
_maxDepositSizeBP,
_vaultMaxDeposits,
_vaultDepositController
);
}
}
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
POC
url: 'http://127.0.0.1:8545',
forking: {
url: "https://eth-mainnet.g.alchemy.com/v2/<key>",
blockNumber: 19185204
}
},*/
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";
const proxyAddress= "0x4852e48215A4785eE99B640CACED5378Cc39D2A4";
const implementationContractName = "OperatorVCS";
await impersonateAccount(ownerOrDeployer);
const impersonatedSigner = await ethers.getSigner(ownerOrDeployer);
console.log("impersonatedSigner",impersonatedSigner.address);
await fundAccount(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");
})
})
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.