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

L-05: `expel` Function Does Not Clear Student's Review Data, Potential for Stale Data Issues

albert

Summary

When a student is expelled via the expel function, their entries in reviewCount and lastReviewTime mappings are not cleared or reset. While isStudent is set to false and they are removed from listOfStudents, their historical review data persists. If other parts of the system (e.g., a faulty graduation check or off-chain processing) were to incorrectly access this data for an expelled student, it could lead to unexpected behavior.

Vulnerability Details

The expel function performs:

// ...
// Remove from listOfStudents
// ...
isStudent[_student] = false;
// reviewCount[_student] and lastReviewTime[_student] are NOT modified.

If a student is expelled, they should not be considered for graduation. The primary guard is isStudent[_student] == false and removal from listOfStudents. However, their reviewCount (e.g., might be 3 if expelled in week 3) remains.

Impact

  • Stale Data Persistence: The review data for expelled students remains in storage.

  • Minor Risk of Misinterpretation (Edge Case): If, due to a separate bug or flawed logic, the system attempts to evaluate an expelled student for graduation (e.g., by iterating an old list of students or if isStudent check is missed elsewhere), their existing reviewCount could be incorrectly used. For example, if H-04's check for reviewCount == 4 were applied to an expelled student who happened to have 4 reviews before expulsion, it might lead to confusion if the isStudent flag wasn't the primary gate.
    This is primarily a data hygiene and defense-in-depth issue. The system should rely on isStudent and the current listOfStudents, but not clearing other associated data is less clean.

Tools Used

Manual Review, Logical Analysis.

Recommendations

Consider clearing the reviewCount and lastReviewTime for an expelled student within the expel function. This ensures that all data related to their active student status is reset, improving data hygiene and reducing the surface for potential misinterpretations by other system components or off-chain tools.

Code Modification for LevelOne.sol::expel:

// src/LevelOne.sol
// ... (other parts of the contract) ...
​
function expel(address _student) public onlyPrincipal {
require(inSession, "HH__NotInSession");
​
if (_student == address(0)) {
revert HH__ZeroAddress();
}
​
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
​
uint256 studentLength = listOfStudents.length;
for (uint256 n = 0; n < studentLength; n++) {
if (listOfStudents[n] == _student) {
listOfStudents[n] = listOfStudents[studentLength - 1];
listOfStudents.pop();
break;
}
}
​
isStudent[_student] = false;
​
// --- START OF MODIFICATION FOR L-05 ---
// Clear review-related data for the expelled student
delete reviewCount[_student];
delete lastReviewTime[_student];
// studentScore[_student] could also be deleted if it's considered sensitive
// or should not persist for expelled students, e.g., delete studentScore[_student];
// --- END OF MODIFICATION FOR L-05 ---
​
emit Expelled(_student);
}
​
// ... (other parts of the contract) ...
Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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