Hawk High

Issue Summary

The LevelOne contract fails to enforce a critical educational requirement: the verification that each student has received exactly 4 reviews (one per week) before system upgrade. This directly contradicts the explicitly stated invariant: "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 - High

The absence of this check compromises the educational integrity of the entire system by allowing students to progress without receiving complete academic evaluation. This creates an inconsistent and potentially unfair academic environment where some students receive full evaluation while others may advance with minimal or no reviews.

Likelihood - High

Given the manual nature of the review process and dependency on teacher actions, it's highly probable that some students will not receive all required reviews. Without an enforcement mechanism, the issue will manifest whenever a graduation cycle occurs with incomplete student reviews.

Technical Details

The contract includes functionality to track and limit reviews:

1. A reviewCount mapping tracks the number of reviews per student:

   mapping(address => uint256) private reviewCount;

2. The giveReview function has a check to prevent excessive reviews:

   require(reviewCount[_student] < 5, "Student review count exceeded!!!");

3. However, this counter is never incremented in the giveReview function, rendering the check ineffective:

   function giveReview(address _student, bool review) public onlyTeacher {

   // ... validation checks ...

   // Missing: reviewCount[_student]++;

   lastReviewTime[_student] = block.timestamp;

   // ...

   }

4. The graduateAndUpgrade function contains no verification of review counts:

   function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {

   // No check for minimum review count

   // ...

   }

This creates a double failure:

1. The review counting mechanism is broken due to not incrementing the counter

2. Even if it worked correctly, the graduation function doesn't enforce the minimum review requirement

The result is a complete breakdown of the review system's integrity, as students can graduate regardless of how many reviews they've received.

Proof of Concept

Consider this sequence of events:

1. A school session starts with 10 students

2. Teachers give reviews to only 5 students

3. At session end, the principal calls graduateAndUpgrade

4. The function executes successfully despite 5 students having received no reviews

5. The invariant "Students must have gotten all reviews" is violated

Remediation

Two key fixes are required:

1. Fix the review counting mechanism in giveReview:

   function giveReview(address _student, bool review) public onlyTeacher {

   if (!isStudent[_student]) {

   revert HH__StudentDoesNotExist();

   }

   require(reviewCount[_student] < 5, "Student review count exceeded!!!");

   require(block.timestamp >= lastReviewTime[_student] + reviewTime, "Reviews can only be given once per week");

   ​

   // Update review count

   reviewCount[_student]++; // Add this line

   ​

   // Remaining function logic...

   }

2. Add review verification in graduateAndUpgrade:

   function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {

   if (_levelTwo == address(0)) {

   revert HH__ZeroAddress();

   }

   // Verify all students have received exactly 4 reviews

   for (uint256 i = 0; i < listOfStudents.length; i++) {

   require(reviewCount[listOfStudents[i]] == 4, "Not all students have received the required 4 reviews");

   }

   // Remaining function logic...

   }

These changes ensure that the educational process is properly tracked and verified before graduation, maintaining the academic integrity specified in the system invariants.