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

Cut off score is not validated on graduate and upgrade

bavoen

Summary

According to the documentation only students that have a score higher than the cut off score are to be upgraded.

  • Any student who doesn't meet the cutOffScore should not be upgraded

The current implementation does not check if the students have a score higher than the cut off score before upgrading them.

Vulnerability Details

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

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

The function does not check if the students have a score higher than the cut off score before upgrading them. This means that all students will be upgraded regardless of their score.

The following test should validate that the students are not upgraded if they do not have a score higher than the cut off score:

function test_students_below_cutoff_are_not_upgraded() public schoolInSession()
{
// four negative reviews will make the score of harriet 60
for (uint256 i = 0; i < 4; i++) {
vm.warp(block.timestamp + 1 weeks);
​
vm.prank(alice);
levelOneProxy.giveReview(harriet, false);
}
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
assertEq(levelTwoProxy.getListOfStudents().length, 5);
}

but this fails since the students are upgraded regardless of their score.

Impact

The current implementation allows students to be upgraded without meeting the required cut off score, which does not meet the documentation requirements.

Tools Used

Manually reviewed the code and the documentation.

Recommendations

The graduateAndUpgrade function should be changed to check if the students have a score higher than the cut off score before upgrading them. This can be done by adding a check for each student in the listOfStudents array and removing those that do not meet the cut off score from the array before upgrading the contract.

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
+ for (uint256 i = 0; i < listOfStudents.length; i++) {
+ if (studentScore[listOfStudents[i]] < cutOffScore) {
+ listOfStudents[i] = listOfStudents[listOfStudents.length - 1];
+ listOfStudents.pop();
+ i--;
+ }
+ }
​
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 6 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 6 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.