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

`LevelOne:graduateAndUpgrade` is not included in `upgradeToAndCall` function, which might result in upgrading contract without disbursing wages

manaxtech

Description: As the name indicates graduateAndUpgrade is meant to disburse wages to principal and teachers then upgrade the contract. But in this case the pricipal might think only calling the function graduateAndUpgrade will disburse the wages and upgrade the contract, or calling upgradeToAndCall function will disburse the wages and upgrade the protocol, but the function that upgrade the contract is in upgradeToAndCall and graduateAndUpgrade function that disburse the wages is not mentioned any where in upgradeToAndCall function.

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;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}

Vulnerability Details: graduateAndUpgrade function is not called in upgradeToAndCall function

Impact: Not disbursing wages when upgradeToAndCall function is called

Tools Used: Manual Review

Proof of Concept: there are two test that show the vulnerability
Add the UUPSUpgradable in LevelTwo contract to make this test suit work

  1. Test suit that shows only calling upgradeToAndCall Function does not disburs wages to principal and teachers

Proof of Code
function test_graduateAndUpgradeIsNotIncludedInUpgradingfunction() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
​
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(eli);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(fin);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(grey);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(harriet);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// Session has not been started
bool hasSessionStarted = levelOneProxy.getSessionStatus();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.startPrank(principal);
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
​
assert(usdc.balanceOf(principal) == 0);
assert(LevelTwo(proxyAddress).TEACHER_WAGE_L2() == 40);
assert(!hasSessionStarted);
}

  1. Test suit that shows calling graduateAndUpgrade does not upgrade protocol

Proof of Code
function test_graduateAndUpgradeDoesNotUpgradeTheProtocol() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
​
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(eli);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(fin);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(grey);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(harriet);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// Session has not been started
bool hasSessionStarted = levelOneProxy.getSessionStatus();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.startPrank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
​
vm.expectRevert();
LevelTwo(proxyAddress).TEACHER_WAGE_L2();
assert(usdc.balanceOf(principal) == 1500 ether);
assert(!hasSessionStarted);
}

Recommendations: make the graduateAndUpgrade function private and add it to upgradeAndCall function

function graduateAndUpgrade(address _levelTwo, bytes memory)
- public
+ private
onlyPrincipal
{
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
// @audit-low: reminder in division
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);
}
function upgradeToAndCall(address newImplementation, bytes memory data) public payable override onlyProxy {
+ graduateAndUpgrade(newImplementation, data);
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, data);
}
Updates

Lead Judging Commences

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