Hawk High

First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

`graduateAndUpgrade` does not validate the session end date

Summary

According to the documentation, the system upgrade should not occur if the school's session end date has not been reached.

  • System upgrade cannot take place unless school's sessionEnd has reached
    However, the graduateAndUpgrade function does not validate the session end date before upgrading the contract.

Vulnerability Details

The vulnerability is located in the graduateAndUpgrade function of the LevelOne contract.

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

There is no validation for the session end date before upgrading the contract. This means that the contract can be upgraded before the session end date has been reached, which is a violation of the documentation.

The existing test test_confirm_can_graduate can be used to confirm this vulnerability. The test tries to upgrade the contract but has no warp statement to simulate the passage of time.

function test_confirm_can_graduate() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
console2.log(levelTwoProxy.bursary());
console2.log(levelTwoProxy.getTotalStudents());
}

This test should fail according to the documentation but it does not.

Impact

Since the teachers and principal are paid on system upgrade, this can lead to teachers and the principal being paid before the session end date has been reached.

Tools Used

Manually reviewed the code and the documentation.

Recommendations

The graduateAndUpgrade function should be changed to validate the session end date before upgrading the contract. This can be done by adding a require statement to check if the session end date has been reached before upgrading the contract.

function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ require(block.timestamp >= sessionEnd, "Session end date has not been reached");
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);
}
Updates

Lead Judging Commences

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

can graduate without session end

`graduateAndUpgrade()` can be called successfully even when the school session has not ended

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

can graduate without session end

`graduateAndUpgrade()` can be called successfully even when the school session has not ended

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.