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

`LevelOne:graduateAndUpgrade` Does Not Perform Actual Upgrade Or Initialization

azriel20005

Description:
LevelOne:graduateAndUpgrade currently calls _authorizeUpgrade(_levelTwo), however this makes an internal call to an empty function. This call does not actually upgrade the implementation contract to the _levelTwo contract nor call the LevelTwo:graduate function to reinitialize the proxy.

Impact:
This results in the protocol never being upgraded properly and will always read from the same implementation contract.

Proof of Concept:
By adding the following test case, we can confirm that the implementation address stored within the proxy contract does not change even when LevelOne:graduateAndUpgrade is called.

function test_upgrade_did_not_upgrade() public schoolInSession {
bytes32 initialImplementation = vm.load(proxyAddress, ERC1967Utils.IMPLEMENTATION_SLOT);
address initialImplementationAddress = address(uint160(uint256(initialImplementation)));
​
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
bytes32 newImplementation = vm.load(proxyAddress, ERC1967Utils.IMPLEMENTATION_SLOT);
address newImplementationAddress = address(uint160(uint256(newImplementation)));
​
assertEq(initialImplementationAddress, newImplementationAddress);
}

Recommended Mitigation:
Use UUPSUpgradeable:upgradeToAndCall function instead and pass in the data to initialize the proxy with the _levelTwo address.

-- function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
++ function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
​
// @audit-info: teachers should share the 35% right? this is doing each teacher is getting paid 35%
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
-- _authorizeUpgrade(_levelTwo);
++ upgradeToAndCall(_levelTwo, data);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}
Updates

Lead Judging Commences

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