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

`LevelOne::graduateAndUpgrade` does not update the implementation

jfornells

Drscription

The graduateAndUpgrade(address _levelTwo, bytes memory) function in the LevelOne contract is intended to update the proxy contract's implementation to LevelTwo while distributing the accumulated balance (bursary) among teachers and the principal.

However, no actual contract update occurs, as the function only calls _authorizeUpgrade(_levelTwo), which merely validates that the msg.sender is authorized to perform the upgrade. This validation alone does not change the proxy's implementation.

As a result, the proxy continues to execute LevelOne logic, and the new implementation (LevelTwo) never comes into effect.

This behavior introduces an additional risk: since the implementation is not changed, the graduateAndUpgrade() function can be called multiple times, redistributing funds from the bursary each time if available.

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

Impact:

  • The upgrade does not occur, so LevelTwo functionalities are never activated.

  • The principal can abuse the function, calling it multiple times and withdrawing funds repeatedly while the old logic remains operational.

  • Users and external systems may believe the contract has been upgraded, but internally it remains LevelOne, creating inconsistencies and operational confusion.

Proof of Concept:

  1. Deploy LevelTwo, which is expected to be the new implementation.

LevelTwo levelTwo = new LevelTwo();

  1. The principal encodes the call to the graduate() function (from LevelTwo) and executes graduateAndUpgrade(...), attempting to update the proxy's implementation.

bytes memory data = abi.encodeWithSignature("graduate()");
LevelOne(proxy).graduateAndUpgrade(address(levelTwo), data);

  1. Call a known function (getPrincipal()) after the supposed upgrade. This function is defined in both LevelOne and LevelTwo.

LevelTwo(proxy).getPrincipal();

  1. The execution trace shows that the contract still performs a delegatecall to the LevelOne implementation, not LevelTwo, proving that the upgrade did not occur:

@> ├─ [560] LevelOne::getPrincipal() [delegatecall]
│ └─ ← [Return] principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca]

This line clearly indicates that the active logic is still LevelOne.

  1. Verify that graduateAndUpgrade() can be executed multiple times, redistributing funds to the principal each time.

LevelOne(proxy).graduateAndUpgrade(address(levelTwo), data);
console2.log("Principal Balance 1rst Time:..", usdc.balanceOf(principal));
// 2.
LevelOne(proxy).graduateAndUpgrade(address(levelTwo), data);
console2.log("Principal Balance 2nd Time:..", usdc.balanceOf(principal));

  1. Log output:

Logs:
Principal Balance 1rst Time:.. 3000000000000000000000
Principal Balance 2nd Time:.. 4500000000000000000000

The principal's balance increased by 1500 USDC from the 1st to the 2nd execution, confirming that:

  • The implementation did not change.

  • The graduateAndUpgrade() function can be called multiple times.

Tools Used:

Manual Review, Foundry

Recommended Mitigation:

To ensure the contract's implementation is correctly updated, replace the _authorizeUpgrade(...) call with a call to _upgradeToAndCall(...), which includes permission validation, performs the effective upgrade, and optionally calls an initialization function of the new implementation (graduate()).

  • The proxy points to the new implementation (LevelTwo).

  • The graduate() function executes correctly.

- function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
+ function graduateAndUpgrade(address _levelTwo, bytes memory _data) public onlyPrincipal {
...
- _authorizeUpgrade(_levelTwo);
+ upgradeToAndCall(address newImplementation, bytes memory _data)
}
Updates

Lead Judging Commences

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