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

Broken UUPS Upgrade Implementation

mishoko

Summary

The LevelOne contract implements the UUPS (Universal Upgradeable Proxy Standard) pattern but fails to properly execute the upgrade mechanism in its graduateAndUpgrade function. The function accepts upgrade parameters but does not invoke the actual upgrade functionality from the OpenZeppelin UUPS library, rendering the contract's upgrade functionality broken.

Vulnerability Details

The graduateAndUpgrade function in LevelOne.sol is intended to upgrade the contract to a new implementation (_levelTwo). However, while it calls _authorizeUpgrade(_levelTwo) to verify authorization, it never invokes the actual upgrade function _upgradeToAndCallUUPS provided by the inherited OpenZeppelin UUPSUpgradeable contract.

In the OpenZeppelin implementation, the upgrade process follows this pattern:

function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, data);
}

However, the LevelOne implementation only performs authorization:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
// ... validation and fund distribution logic ...
_authorizeUpgrade(_levelTwo);
// Missing: _upgradeToAndCallUUPS(_levelTwo, data) or equivalent
// ... more fund distribution ...
}

This means that while the contract appears to support upgrades, the actual upgrade mechanism is never triggered.

Impact

The vulnerability has several significant impacts:
invariant broken: "At the end of the school session (4 weeks), the system is upgraded to a new one."

  1. Broken Upgradability: The contract cannot be upgraded despite appearing to support upgrades, which will lock the system in its current implementation indefinitely.

  2. False Security Assumption: Users and administrators may falsely believe they can upgrade the contract

Proof of Concept

  1. Deploy the LevelOne contract

  2. Call graduateAndUpgrade with a valid new implementation address and data

  3. Observe that while the function executes without errors, the contract's implementation remains unchanged

  4. Verify by checking ERC1967Utils.getImplementation() which will still point to the original implementation

Tools Used

Manual code review

Recommendations

  1. Modify the graduateAndUpgrade function to properly call the UUPS upgrade mechanism:

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;
// Distribute funds before the upgrade
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
// Authorize and perform the upgrade
_authorizeUpgrade(_levelTwo);
// Call the upgradeToAndCall function from UUPSUpgradeable
// This need to be called directly because _upgradeToAndCallUUPS is private
UUPSUpgradeable(address(this)).upgradeToAndCall(_levelTwo, data);
emit Graduated(_levelTwo);
}

  1. Alternatively, ensure the contract inherits and correctly uses the OpenZeppelin UUPS upgrade pattern by leveraging the existing upgradeToAndCall function directly instead of reimplementing it with graduateAndUpgrade.

Updates

Lead Judging Commences

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

Give us feedback!