Hawk High

Summary

The enroll() function in LevelOne.sol is vulnerable to a reentrancy attack. The external call usdc.safeTransferFrom() is made before critical state isStudent[msg.sender] is updated. A malicious USDC token contract (or a user calling via a malicious contract) could re-enter enroll(), allowing the same student to enroll and pay fees multiple times before the session starts.

Vulnerability Details

The enroll() function executes operations in the following order:

1. Checks notYetInSession (passes if session not started).

2. Checks isTeacher or principal (fails if sender is, which is fine).

3. Checks isStudent[msg.sender] (passes if student not yet enrolled).

4. usdc.safeTransferFrom(msg.sender, address(this), schoolFees); (External call)

5. listOfStudents.push(msg.sender);

6. isStudent[msg.sender] = true; (State update to prevent re-enrollment)

7. studentScore[msg.sender] = 100;

8. bursary += schoolFees;

If the usdc.safeTransferFrom() call allows reentrancy (e.g., the token is a contract that calls back into enroll()), the re-entrant call will find:

• notYetInSession is still true.

• isStudent[msg.sender] is still false because the original call hasn't reached step 6.
  Thus, the re-entrant call will pass all checks and execute again.

Impact

1. Multiple Enrollments & Fee Payments: A student can enroll multiple times, paying schoolFees each time, as long as the session has not started.

2. Inflated Bursary: The bursary will be artificially inflated by the multiple fee payments.

3. Duplicate Entries in listOfStudents: The listOfStudents array will contain duplicate addresses for the re-enrolling student. This could affect any logic that relies on this list for unique student counts or iteration, potentially leading to incorrect calculations or behavior in other parts of the system or in off-chain processing.

4. Violation of Implicit Uniqueness: The HH__StudentExists error implies that a student should only exist once. While the isStudent mapping correctly prevents re-enrollment after the first successful full execution, the reentrancy circumvents this during the execution of the first call.

5. Waste of Gas for the User: The user pays gas for multiple enrollments.

While the studentScore is reset and the isStudent mapping eventually gets set to true, the duplicated entries in listOfStudents and the inflated bursary are undesirable side effects.

Tools Used

Manual Review, Logical Analysis (Checks-Effects-Interactions pattern).

Recommendations

Follow the Checks-Effects-Interactions pattern. Update all relevant state variables before making external calls. Specifically, set isStudent[msg.sender] = true before the usdc.safeTransferFrom() call. Alternatively, implement a reentrancy guard modifier.

Code Modification for LevelOne.sol::enroll() (Preferred: Update state first):

Alternative using Reentrancy Guard (less intrusive on logic order if preferred, but adds a dependency):

The first approach (updating state before external call for isStudent) is generally preferred for mitigating reentrancy related to "already processed" checks. A full reentrancy guard protects the entire function. Given the specific vulnerability path, setting isStudent[msg.sender] = true; early effectively acts as a specific lock for this reentrancy path.