Hawk High

Summary

The LevelOne contract fails to utilize the cutOffScore established by the principal when the graduateAndUpgrade function is called. According to the project invariants, "Any student who doesn't meet the cutOffScore should not be upgraded." However, the current implementation of graduateAndUpgrade proceeds with the system upgrade and wage payments without checking any student's score against the cutOffScore, thus not differentiating between students who meet the academic requirements and those who do not.

Vulnerability Details

The project explicitly defines an invariant: "Any student who doesn't meet the cutOffScore should not be upgraded." This implies that the cutOffScore (set by the principal via startSession(uint256 _cutOffScore)) should serve as a threshold for determining student eligibility for progression to the next level or for being considered "graduated" in the context of the system upgrade.

The LevelOne.graduateAndUpgrade(address _levelTwo, bytes memory) function currently:

1. Authorizes the upgrade to _levelTwo.

2. Calculates and pays wages to teachers and the principal based on the bursary.

It does not contain any logic to:

• Iterate through the listOfStudents.

• For each student, compare their studentScore[studentAddress] with the cutOffScore.

• Take any differential action based on this comparison. For example, it doesn't prevent the upgrade if some students fail, nor does it mark students as "graduated" or "not graduated" based on this score.

The term "upgraded" in the invariant ("should not be upgraded") is somewhat ambiguous in the current contract structure, as the upgrade is a system-wide contract pointer change rather than an individual student status change. However, the intent is clear: the cutOffScore should play a decisive role in the outcome related to students at the end of the session. The current contract completely ignores this parameter during the critical graduateAndUpgrade phase.

Impact

The failure to enforce the cutOffScore has several negative consequences:

1. Violation of Academic/Business Rule:

   • A core rule of the Hawk High school ("students must meet a cutoff to graduate/be upgraded") is not implemented, directly breaching a stated invariant. This means the system does not operate as specified.

2. No Consequence for Underperformance:

   • Students who do not meet the academic standard (as defined by cutOffScore) face no in-contract consequence. The studentScore and cutOffScore variables become informational only, without any functional impact on the graduation/upgrade process.

3. Diminished Value of Scoring System:

   • The entire mechanism of teachers giving reviews and students accumulating scores is undermined if the final score doesn't influence a key outcome like graduation or progression.

4. Potential Misalignment with LevelTwo Expectations:

   • If LevelTwo is designed with the expectation that only "qualified" students are implicitly part of its cohort (e.g., if student lists were to be migrated or if access to LevelTwo functions depended on a "graduated" status from LevelOne), this lack of filtering in LevelOne could cause issues.

5. Erosion of Perceived Fairness and Standards:

   • If all students are treated the same by the graduateAndUpgrade process regardless of their scores, it may be perceived as unfair or as a lowering of academic standards by the participants.

Tools Used

manual review

Recommendations

1. Define "Upgraded" Semantics for Students:

   • Clarify precisely what "should not be upgraded" means for a student.

     • Option A: Prevent System Upgrade: If any student fails to meet the cutOffScore (and has received all reviews), should the entire system upgrade be blocked until the principal addresses this (e.g., by expelling failing students or some other defined process)? This is a strict interpretation.

     • Option B: Mark Student Status: Implement a mapping or array to track the "graduation status" of each student (e.g., mapping(address => bool) public isGraduated;). In graduateAndUpgrade, iterate through students, check their score against cutOffScore, and set their isGraduated status accordingly. The system upgrade itself would still proceed.

     • Option C: Filter Student List for Next Level: If LevelTwo is meant to inherit a list of students, only students meeting the cutOffScore should be part of this list passed to or recognized by LevelTwo. This would require a mechanism to pass this filtered list (e.g., via calldata during upgrade, or by LevelTwo reading a specific state from LevelOne).

2. Implement CutOffScore Check in graduateAndUpgrade:

   • Based on the chosen semantic (from point 1), modify LevelOne.graduateAndUpgrade:

   • Before authorizing the upgrade or paying wages, iterate through listOfStudents.

   • For each student, retrieve studentScore[studentAddress].

   • Compare it with cutOffScore.

   • Also, ensure the student has received the required number of reviews (e.g., 4) before checking their score against the cutoff. (This links to fixing the reviewCount bug and enforcing the review count invariant).

   • Apply the decided logic (block upgrade, set status, filter list).

   // Example for Option B (Marking Status) in LevelOne.graduateAndUpgrade

   // This assumes reviewCount bug is fixed and there's a way to check it.

   // mapping(address => bool) public isGraduated; // New state variable

   // uint256 public constant REQUIRED_REVIEWS = 4; // New constant

   ​

   // ... inside graduateAndUpgrade, before _authorizeUpgrade ...

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

   address student = listOfStudents[i];

   // require(reviewCount[student] == REQUIRED_REVIEWS, "Student has not received all reviews"); // Add this check

   if (studentScore[student] >= cutOffScore && reviewCount[student] == REQUIRED_REVIEWS) {

   isGraduated[student] = true;

   // emit StudentGraduated(student); // Optional: new event

   } else {

   isGraduated[student] = false;

   // emit StudentDidNotGraduate(student); // Optional: new event

   }

   }

   // Now proceed with _authorizeUpgrade and payments.

   // LevelTwo might then read the isGraduated mapping if needed.
   
   

3. Consider Impact on Wage Payment:

   • Decide if the graduation status of students should impact the bursary portion used for wages. The current invariant ("principal gets 5% of bursary", "teachers share of 35% of bursary") implies wages are based on the total collected fees, regardless of how many students graduate. If this needs to change, the invariant and logic must be updated.

4. Documentation and Events:

   • Clearly document how the cutOffScore affects students and the upgrade process.

   • Emit events related to student graduation status if implementing Option B or C.

5. Thorough Testing:

   • Test scenarios with students above, at, and below the cutOffScore.

   • Test interaction with the review count requirement.

   • Test the chosen "upgraded" semantic to ensure it behaves as expected.