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

[H-2] Function `LevelOne::graduateAndUpgrade` doesn't check if students have 4 reviews, breaking the invariant of not being able to upgrade unless all students have 4 reviews

sa1nt

Description: In function LevelOne::graduateAndUpgrade there is no check for the amount of reviews students have at the end of the session, making it able for the protocol to be upgraded even if there are students who do not have all 4 reviews. This is breaking the invariant that is specified in the protocol's documentation.

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)

Impact: The protocol invariant is broken, giving the possibility for the principal to upgrade the protocol even if not all students have 4 reviews.

Proof of Concept: As we can see in the test the graduateAndUpgrade function doesn't revert even if all students have 0 reviews.

function testCanGraduateAndUpgradeEvenIfStudentsDontHaveFourReviews() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
// doesn't revert because there is no check on the number of reviews for students
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
}

Recommended Mitigation: Modify the LevelOne::giveReview and LevelOne::enroll functions to increment and track the total number of students fully reviewed, so that graduateAndUpgrade only needs to check a single counter.

Add a new state variable:

+ uint256 public fullyReviewedStudents;

In the giveReview function, increment the reviewCount[_student] and when the student reaches exactly 4 reviews, increment fullyReviewedStudents:

+ reviewCount[_student] += 1;
+ if (reviewCount[_student] == 4) {
+ fullyReviewedStudents += 1;
+ }

In graduateAndUpgrade, simply check:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ require(fullyReviewedStudents == listOfStudents.length, "Not all students have 4 reviews");
.
.
.
}

In LevelOne::expel, if a student who already had 4 reviews is expelled, decrement fullyReviewedStudents so the counter stays correct:

+ if (reviewCount[_student] >= 4) {
+ fullyReviewedStudents -= 1;
+ }
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.