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

Missing upgradeTo Call Results in Incomplete Upgrade Execution

sekkiat

Summary

The graduateAndUpgrade function currently verifies authorization for the upgrade but does not invoke the upgrade itself. Without a call to upgradeTo, the new implementation address is not set, and the contract remains on the old version despite appearing to initiate an upgrade.

Vulnerability Details

The graduateAndUpgrade function is intended to allow the principal to authorize and execute an upgrade to a new implementation contract. While the function includes a call to _authorizeUpgrade(_levelTwo), which verifies that the caller has permission to initiate an upgrade, it fails to include the actual upgrade execution through a call to upgradeTo(_levelTwo). As a result, even if the caller is authorized, the contract does not transition to the new implementation, effectively rendering the upgrade incomplete.

Impact

The failure to execute the upgrade means that any expected logic changes or patches in the new contract version will not take effect. This can lead to unexpected behavior, stale code execution, and a false sense of security, especially in critical upgrade paths. In a worst-case scenario, it could leave known vulnerabilities unpatched despite appearing to have performed an upgrade.

Tools Used

Manual Review

Recommendations

To properly execute the upgrade, the call to upgradeTo(_levelTwo) should be made directly, as it already handles the authorization check internally.

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
if (totalTeachers == 0) {
revert("No teachers available for payment");
}
​
uint256 totalTeacherPay = (bursary * TEACHER_WAGE) / PRECISION;
uint256 payPerTeacher = totalTeacherPay / totalTeachers;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
upgradeTo(_levelTwo); // Only this is needed as upgradeTo will also run the _authorizeUpgrade check
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}
​

This guarantees that once authorization is confirmed, the contract transitions to the new implementation address.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

failed upgrade

The system doesn't implement UUPS properly.

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