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

`LevelOne:graduateAndUpgrade` Does Not Filter Students Below Cutoff

azriel20005

Description:
Students who do not meet the cutoff score are still upgraded to LevelTwo as there are no checks to filter them out.

Impact:
This violates the invariant "Any student who doesn't meet the cutOffScore should not be upgraded"

Proof of Concept:

function test_students_below_cutoff_not_removed() public schoolInSession {
vm.warp(block.timestamp + 1 weeks);
vm.prank(alice);
levelOneProxy.giveReview(harriet, false);
​
assertEq(levelOneProxy.getTotalStudents(), 6); // 6 students present initially
assertEq(levelOneProxy.studentScore(harriet), 90);
assertEq(levelOneProxy.cutOffScore(), 100);
assertLt(levelOneProxy.studentScore(harriet), levelOneProxy.cutOffScore()); // assert that harriet did not meet the cutoff score
​
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.getTotalStudents(), 6); // all 6 students are still present
}

Recommended Mitigation:
There are many ways to include this logic, depending on the required logic. The following shows the most straight forward way which is to call expel the students that did not meet the cutoff score, then call upgrade. Other ways that do not use expel would also work if expel is not the desired outcome.

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