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

[H-3] Function `LevelOne::graduateAndUpgrade` doesn't check if there are any students who have not passed, breaking the invariant of students not being able to graduate if they don't meet the `cutOffScore`

sa1nt

Description: In function LevelOne::graduateAndUpgrade there is no check for students who do not have the minimum amount of score to be able to graduate, meaning that the protocol can be upgraded even if there are students who have not passed. These students should be expelled before upgrading the protocol so students who do not pass can't graduate. This breaks one of the invariants of the protocol that is written in the documentation of the protocol.

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

Impact: Students who do not meet the cutOffScore can be graduated by the system, breaking the protocol.

Proof of Concept: Student eli has a score of 60 which does not meet the cutOffScore of 70, but function graduateAndUpgrade does not revert.

Put this in the LeveOnelAndGraduateTest.t.sol:

function testStudentWhoDoesNotMeetCutOffScoreCanGraduate() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
for (uint256 i = 0; i < 4; i++) {
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(alice);
levelOneProxy.giveReview(harriet, true);
levelOneProxy.giveReview(dan, true);
levelOneProxy.giveReview(fin, true);
levelOneProxy.giveReview(grey, true);
levelOneProxy.giveReview(clara, true);
// student who does not meet cut off score
levelOneProxy.giveReview(eli, false);
vm.stopPrank();
}
​
uint256 score = levelOneProxy.studentScore(eli);
assertEq(score, 60); // lower than the cut off score of 70
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data); // does not revert
}

Recommended Mitigation: Keep track of the students who have their score below the cutOffScore in the LevelOne::giveReview function after every review given and check in the graduateAndUpgrade function if the number is greater than 0.

Add a variable to keep track of number of students who have not passed:

+ uint256 cutOffStudents;

Check if a student's score has gone below the threshold:

function giveReview(address _student, bool review) public onlyTeacher {
.
.
.
// where `false` is a bad review and true is a good review
if (!review) {
studentScore[_student] -= 10;
}
+ if (studentScore[_student] < cutOffScore) {
+ cutOffStudents++;
+ }
.
.
.
}

Revert if cutOffStudents is bigger than 0 when trying to upgrade protocol:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ if (cutOffStudents > 0) {
+ revert();
+ }
.
.
.
}
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.

Support

FAQs

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