Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

`enroll` Function in `Session1.sol` Violates CEI (Checks-Effects-Interactions) Principle

bambooboo

Summary

The enroll function in Session1.sol performs external interactions before updating the contract’s internal state, violating the Checks-Effects-Interactions (CEI) pattern. This could potentially open up the function to reentrancy-like issues and lead to inconsistent state.

Vulnerability Details

The enroll function is implemented as follows:

Source Link (Line 143)

function enroll() external notYetInSession {
if (isTeacher[msg.sender] || msg.sender == principal) {
revert HH__NotAllowed();
}
if (isStudent[msg.sender]) {
revert HH__StudentExists();
}
usdc.safeTransferFrom(msg.sender, address(this), schoolFees);
​
listOfStudents.push(msg.sender);
isStudent[msg.sender] = true;
studentScore[msg.sender] = 100;
bursary += schoolFees;
​
emit Enrolled(msg.sender);
}

This function first performs an external call to usdc.safeTransferFrom, and only after that updates critical internal state variables such as isStudent, listOfStudents, and studentScore.

If the usdc contract is malicious or contains a callback mechanism, it could re-enter the enroll function before the internal state has been updated. Since the student isn’t marked as enrolled yet, the second call could succeed again, leading to:

  • Duplicate entries in listOfStudents

  • Unintended financial or logical behavior when managing students (e.g., during expulsion)

While this may not be exploitable in the current setup, following the CEI pattern is a critical best practice to mitigate similar risks and ensure robust contract logic.

Impact

  • Data Inconsistency: Duplicate entries in listOfStudents with conflicting isStudent values

  • Potential Reentrancy Risk: Reentrancy vector enabled via external usdc call

  • Logical Vulnerabilities: Unexpected behavior during expulsion or reward logic

Tools Used

  • Manual Code Review

Recommendations

Refactor the function to follow the Checks-Effects-Interactions pattern:

  1. Perform all checks first.

  2. Update internal state.

  3. Then perform external interactions.

  4. Emit events last.

Proposed Fix:

function enroll() external notYetInSession {
if (isTeacher[msg.sender] || msg.sender == principal) {
revert HH__NotAllowed();
}
if (isStudent[msg.sender]) {
revert HH__StudentExists();
}
​
// Update internal state before external call
listOfStudents.push(msg.sender);
isStudent[msg.sender] = true;
studentScore[msg.sender] = 100;
bursary += schoolFees;
​
usdc.safeTransferFrom(msg.sender, address(this), schoolFees);
​
emit Enrolled(msg.sender);
}

This ensures state consistency even if the external call behaves unexpectedly.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!