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

Broken UUPS Upgrade Implementation Prevents Correct Contract Transition

howiecht

Summary

The graduateAndUpgrade() function incorrectly uses the _authorizeUpgrade internal hook instead of the standard upgradeToAndCall function, and the Level Two contract is not properly configured for UUPS. This prevents the proxy contract from upgrading correctly to the Level Two implementation, failing the intended transition.

Vulnerability Details

The contract attempts to use the UUPS (Universal Upgradeable Proxy Standard) pattern for upgrades. However, the graduateAndUpgrade() function in the LevelOne contract misuses the _authorizeUpgrade function. In the UUPS pattern, _authorizeUpgrade is designed as an internal hook that is called by the standard upgrade functions like upgradeTo or upgradeToAndCall (provided by OpenZeppelin's UUPSUpgradeable base contract) to perform access control or other pre-upgrade checks. It is not the function intended to trigger the upgrade process itself.

The problematic line is:

// Incorrectly calling the internal authorization hook directly
_authorizeUpgrade(_levelTwo);

Furthermore, for the UUPS pattern to work correctly, the new implementation contract (LevelTwo) must also inherit from UUPSUpgradeable and provide its own implementation of the _authorizeUpgrade hook (even if empty, as shown in the recommended fix). The original LevelTwo contract did not appear to do this.

This misuse means the OpenZeppelin UUPS proxy logic, which handles updating the proxy's implementation address, is never invoked correctly. The test case provided demonstrates that without the fix, the intended state of Level Two is not reached:

function test_confirm_can_graduate() public schoolInSession {
levelTwoImplementation = new LevelTwo(); // Assumes original LevelTwo without UUPS inheritance
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
// This call executes, but uses _authorizeUpgrade directly,
// failing to trigger the proxy's UUPS upgrade logic.
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
// This assertion would likely fail because levelOneProxy (proxyAddress)
// is still pointing to the old LevelOne implementation, not the LevelTwo one.
uint256 levelTwoTeacherWage = 40;
assertEq(LevelTwo(proxyAddress).TEACHER_WAGE_L2(), levelTwoTeacherWage);
}
​

The assertion assertEq(LevelTwo(proxyAddress).TEACHER_WAGE_L2(), levelTwoTeacherWage); would fail because proxyAddress is still directing calls to the LevelOne implementation, which does not have the TEACHER_WAGE_L2() function or the expected state of the Level Two contract.

Impact

The proxy contract's implementation address is not updated to point to the new Level Two contract. Any calls made through the proxy after the failed upgrade attempt will continue to execute the code of the Level One implementation, not the Level Two implementation.

Tools Used

Recommendations

To correctly implement the UUPS upgrade using OpenZeppelin's libraries, modify the graduateAndUpgrade() function to call upgradeToAndCall() instead of _authorizeUpgrade(). Additionally, ensure the LevelTwo contract correctly inherits from UUPSUpgradeable and implements the _authorizeUpgrade hook.

function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
- _authorizeUpgrade(_levelTwo);
+ upgradeToAndCall(_levelTwo, data);
​
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}
- contract LevelTwo is Initializable {
+ contract LevelTwo is Initializable, UUPSUpgradeable{
// ...skip
+ function _authorizeUpgrade(address newImplementation) internal override {}
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 2 months 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.