Summary
This report highlights a vulnerability in the graduateAndUpgrade function of the LevelOne contract, which prevents the upgrade of the proxy contract’s implementation to the LevelTwo contract. The issue lies in the missing call to OpenZeppelin's upgradeTo method for performing the upgrade, leaving the proxy unchanged and causing the upgrade mechanism to fail.
Vulnerability Details
The function graduateAndUpgrade in the LevelOne contract attempts to upgrade the proxy to a new implementation (LevelTwo). However, it fails to call the required OpenZeppelin upgradeTo function that would update the proxy's implementation address. Without this, the proxy continues pointing to the old LevelOne implementation.
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
_authorizeUpgrade(_levelTwo);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}
The problem lies in the fact that the function attempts to authorize the upgrade using _authorizeUpgrade but does not actually call the necessary upgrade mechanism (i.e., upgradeTo) to update the proxy’s implementation.
Proof Of Concept (POC)
The following test script was used to verify this issue:
function test_graduateAndUpgrade() public {
console2.log("levelOne Implementation Address", getImplementationAddress());
console2.log("Proxy Address", getProxyAddress());
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
console2.log("LevelTwo Implementation Address", levelTwoImplementationAddress);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
LevelTwo levelTwoProxy = LevelTwo(address(levelOneProxy));
levelTwoProxy.getTotalStudents();
levelTwoProxy.getListOfTeachers();
}
This Test Script Output
Traces:
[360427] LevelOneAndGraduateTest::test_graduateAndUpgrade()
├─ [0] console::log("levelOne Implementation Address", LevelOne: [0x90193C961A926261B756D1E5bb255e67ff9498A1]) [staticcall]
│ └─ ← [Stop]
├─ [2365] DeployLevelOne::getProxyAddress() [staticcall]
│ └─ ← [Return] ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496]
├─ [0] console::log("Proxy Address", ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496]) [staticcall]
│ └─ ← [Stop]
├─ [228469] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 1141 bytes of code
├─ [0] console::log("LevelTwo Implementation Address", LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [25188] ERC1967Proxy::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [20283] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [7288] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 0)
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 0)
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [2750] ERC1967Proxy::getTotalStudents() [staticcall]
│ ├─ [2360] LevelOne::getTotalStudents() [delegatecall]
│ │ └─ ← [Return] 0
│ └─ ← [Return] 0
├─ [1082] ERC1967Proxy::getListOfTeachers() [staticcall]
│ ├─ [689] LevelOne::getListOfTeachers() [delegatecall]
│ │ └─ ← [Return] []
│ └─ ← [Return] []
└─ ← [Stop]
As shown in the trace, the proxy address remains unchanged, and no upgrade is performed. The function successfully delegates the call to LevelOneBut it does not trigger the implementation upgrade. Note it the script provided in the protocol only calls the levelOne functions. It does not invoke any internal functions related to the update defined in the UUPS. Therefore, as indicated in the contract, the upgrade function was not actually called, meaning the protocol was never upgraded.
Impact
The vulnerability results in the failure to upgrade the proxy's implementation. As a result, the proxy continues interacting with the old LevelOne implementation instead of the new LevelTwo. This leads to the system failing to access any new logic or features implemented in the upgraded contract, potentially affecting any business logic tied to the upgrade.
Tools Used
Manual Review And Foundry Framework
Recommendations
To fix the issue, you should ensure that the graduateAndUpgrade function calls OpenZeppelin's _upgradeTo method to actually perform the upgrade. Here's the corrected version:
function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
_authorizeUpgrade(_levelTwo);
_upgradeTo(_levelTwo);
(bool success, ) = _levelTwo.call(data);
require(success, "Initialization failed");
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}
