Hawk High

Summary

The LevelOne contract's graduateAndUpgrade function does not verify if the current block timestamp has reached or surpassed the sessionEnd timestamp. This violates the project invariant: "System upgrade cannot take place unless school's sessionEnd has reached." Consequently, the principal can prematurely trigger the system upgrade and associated wage payments before the official 4-week session duration has concluded.

Vulnerability Details

The project specifies a clear operational invariant: "System upgrade cannot take place unless school's sessionEnd has reached." The sessionEnd is set during the startSession function as block.timestamp + 4 weeks. This timestamp marks the official conclusion of the academic session.

The LevelOne.graduateAndUpgrade(address _levelTwo, bytes memory) function, which is onlyPrincipal controlled, performs the system upgrade and disburses wages. However, this function lacks any check against sessionEnd:

• There is no require(block.timestamp >= sessionEnd, "Session has not ended yet"); or similar condition.

As a result, the principal can call graduateAndUpgrade at any point after startSession has been called, regardless of whether the 4-week session period has actually elapsed.

Impact

Allowing the system to be upgraded prematurely has several adverse impacts:

1. Violation of Core Timing Invariant:

   • The explicit rule dictating when an upgrade can occur is breached. This means the system's lifecycle is not operating as designed.

2. Premature Session Termination:

   • Students may not have received all their weekly reviews (e.g., if the upgrade happens after 1 week instead of 4). This also clashes with the invariant: "Students must have gotten all reviews before system upgrade."

   • The learning period is cut short, potentially affecting the educational experience.

3. Unfair Wage Calculation (Potentially):

   • While wages are calculated on the bursary, if the bursary itself is expected to grow throughout the 4-week session (e.g., if late enrollments were permitted, though not currently in the code), a premature upgrade would mean wages are calculated on a smaller-than-expected pool. (This is less of an issue with the current enroll function being notYetInSession).

   • More significantly, teachers are paid for a full session's work that might not have been completed.

4. Disruption to Student Progression and Records:

   • If studentScore and other metrics are meant to be finalized over 4 weeks, a premature upgrade interrupts this.

5. Potential for Abuse by Principal:

   • A malicious or negligent principal could trigger the upgrade early for various reasons, such as accessing their wages sooner or avoiding further operational duties for the remainder of the session, to the detriment of students and teachers.

6. Inconsistency with Off-Chain Expectations:

   • Users (students, teachers) will expect the session to last 4 weeks as per the system's design. A premature on-chain upgrade would create confusion and distrust.

Tools Used

manual review

Recommendations

To ensure that the system upgrade only occurs after the designated session duration and to comply with the invariant "System upgrade cannot take place unless school's sessionEnd has reached," the following changes are recommended for LevelOne.graduateAndUpgrade:

1. Add sessionEnd Timestamp Check:

   • At the beginning of the graduateAndUpgrade function, implement a require statement to verify that the current block.timestamp is greater than or equal to the sessionEnd timestamp.

   • Provide a clear error message if the condition is not met.

   // In LevelOne.sol

   // ... (errors) ...

   error HH__SessionNotYetEnded();

   ​

   // ... (inside graduateAndUpgrade function) ...

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

   if (block.timestamp < sessionEnd) {

   revert HH__SessionNotYetEnded();

   }

   // ... rest of the existing graduateAndUpgrade logic ...

   }
   
   

2. Ensure sessionEnd is Reliably Set:

   • Verify that sessionEnd is correctly and reliably set in the startSession function (e.g., sessionEnd = block.timestamp + 4 weeks;). The current implementation seems correct in this regard.

3. Coordinate with Review Count Invariant:

   • The sessionEnd check is one part of ensuring a session completes. This should be complemented by the invariant requiring all students to have 4 reviews. While the sessionEnd ensures time has passed, the review count ensures activity has occurred. Both are important.

   • Consider if the graduateAndUpgrade function should also explicitly check that all students have received 4 reviews, in addition to the sessionEnd check. The current invariant "Students must have gotten all reviews before system upgrade" suggests this dual check.

4. Testing:

   • Test attempts to call graduateAndUpgrade before sessionEnd is reached, ensuring the transaction reverts with the appropriate error.

   • Test successful calls to graduateAndUpgrade at or after sessionEnd.

   • Test scenarios around the exact moment of sessionEnd (e.g., block.timestamp == sessionEnd).

By implementing the sessionEnd check, the contract will robustly enforce the intended 4-week session duration before any upgrade process can be initiated, aligning with the project's operational rules and protecting the integrity of the session lifecycle.