Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Missing Upgrade Execution in `graduateAndUpgrade` Function

bambooboo

Summary

The graduateAndUpgrade function authorizes an upgrade but does not execute the upgrade, leaving the system running on the old implementation despite the function’s name suggesting otherwise.

Vulnerability Details

The graduateAndUpgrade function is shown below:

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);
}

While _authorizeUpgrade(_levelTwo) ensures that the caller is allowed to perform an upgrade, it does not actually perform the upgrade. In UUPSUpgradeable contracts, an upgrade must be explicitly triggered using upgradeTo or upgradeToAndCall.

Since upgradeToAndCall is not invoked, the contract remains on the old implementation, despite its appearance of performing an upgrade. This introduces confusion and could lead to serious deployment issues if the contract is assumed to be running upgraded logic when it is not.

PoC

function test_not_upgrade_successfully() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
vm.expectRevert();
console2.log(levelTwoProxy.TEACHER_WAGE_L2());
}

Impact

  • Upgrade to _levelTwo is not executed

  • The contract stays on the old implementation

  • Misleading function behavior

  • Possible failure to patch vulnerabilities or activate new logic

Tools Used

  • Manual code review

  • OpenZeppelin UUPSUpgradeable documentation

Recommendations

To properly execute the upgrade, modify the function to include a call to upgradeToAndCall at the end. Update the function to accept the bytes memory data argument and invoke the upgrade:

function graduateAndUpgrade(address _levelTwo, bytes memory data) 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);
​
// Execute the upgrade
upgradeToAndCall(_levelTwo, data);
}

This ensures that the upgrade is not only authorized but also executed with any optional initialization logic included in data.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

failed upgrade

The system doesn't implement UUPS properly.

Support

FAQs

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