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

`graduateAndUpgrade` does not validate the student reviews.

bavoen

Summary

According to the documentation, students must have received all reviews before the system upgrade.

  • Students must have gotten all reviews before system upgrade. System upgrade should not occur if any student has not gotten 4 reviews (one for each week)

However, the graduateAndUpgrade function does not validate the student reviews 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 student reviews before upgrading the contract. This means that the contract can be upgraded before all students have received their reviews, 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 without giving 4 reviews to every student.

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

The principal can graduate and upgrade the contract before all students have received their reviews. Since reviews can only decrease the score of a student, this can lead to students graduating without receiving all the expected reviews.

Tools Used

Manually reviewed the code and the documentation.

Recommendations

The graduateAndUpgrade function should be changed to validate the student reviews before upgrading the contract. This will ensure that all students have received their reviews before the contract is upgraded.

​
function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
+ for (uint256 n = 0; n < listOfStudents.length; n++) {
+ if (reviewCount[listOfStudents[n]] < 4) {
+ revert ("student review count not met");
+ }
+ }
​
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:

cut-off criteria not applied

All students are graduated when the graduation function is called as the cut-off criteria is not applied.

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

cut-off criteria not applied

All students are graduated when the graduation function is called as the cut-off criteria is not applied.

Support

FAQs

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