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

`LevelOne::graduateAndUpgrade` does not upgrade implementation contract to `LevelTwo`, causing students to be unable to graduate

37h3rn17y2

Summary

LevelOne::graduateAndUpgrade does not change the implementation address to LevelTwo contract, resulting in the school system remaining in LevelOne, causing students to be unable to graduate and breaking core protocol functionality

Vulnerability Details

ERC1967 contains upgradeToAndCall and _setImplementation functions which helps to change the implementation address of the proxy. However, these functions are not called in LevelOne::graduateAndUpgrade, resulting in the proxy contract still pointing to the LevelOne implementation contract instead of LevelTwo implementation contract.

PoC

Place the following into LevelOne|AndGraduateTest.t.sol and run

forge test --mt testGraduateDoesNotGoToLevelTwo

function testGraduateDoesNotGoToLevelTwo() public {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
// Functions exist in LevelOne but not LevelTwo
// Should revert if upgraded to LevelTwo
// But does not revert, indicating it is still in LevelOne
LevelOne(address(levelTwoProxy)).getSchoolFeesCost();
LevelOne(address(levelTwoProxy)).getSessionStatus();
LevelOne(address(levelTwoProxy)).getSessionEnd();
}

Impact

Impact: High, students are unable to graduate, breaking core protocol functionality
Likelihood: High, principal will upgrade school system at the end of school session (after 4 weeks)
Severity: High

Tools Used

Manual review

Recommendations

Make the following modifications

LevelOne::graduateAndUpgrade

+ function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
- function graduateAndUpgrade(address _levelTwo, bytes memory) 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;
​
+ upgradeToAndCall(_levelTwo, data);
- _authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}

LevelTwo

+ import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
.
.
.
+ modifier onlyPrincipal() {
+ if (msg.sender != principal) {
+ revert HH__NotPrincipal();
+ }
+ _;
+ }
.
.
.
+ contract LevelTwo is Initializable, UUPSUpgradeable {
- contract LevelTwo is Initializable {
.
.
.
+ function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
Updates

Lead Judging Commences

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

failed upgrade

The system doesn't implement UUPS properly.

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

failed upgrade

The system doesn't implement UUPS properly.

Appeal created

37h3rn17y2 Submitter
6 months ago
yeahchibyke Lead Judge
6 months ago
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.