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

No Safeguard Against Removing All Teachers During Active Session

linxun

Summary

The removeTeacher function in the LevelOne contract allows the principal to remove teachers at any time, even during an active school session. If all teachers are removed while a session is in progress, the contract will enter a dysfunctional state as students cannot receive required reviews, preventing them from graduating.

Vulnerability Details

The removeTeacher function can be called by the principal at any time:

// @audit-low if procipal remove all the teacher while the session begin, there gonna be stuck because no one can review the student.
function removeTeacher(address _teacher) public onlyPrincipal {
if (_teacher == address(0)) {
revert HH__ZeroAddress();
}
​
if (!isTeacher[_teacher]) {
revert HH__TeacherDoesNotExist();
}
​
uint256 teacherLength = listOfTeachers.length;
for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
break;
}
}
​
isTeacher[_teacher] = false;
​
emit TeacherRemoved(_teacher);
}

According to the project's invariants, students must receive reviews from teachers:

  • "Students must have gotten all reviews before system upgrade"

  • "System upgrade should not occur if any student has not gotten 4 reviews (one for each week)"

However, if the principal removes all teachers during an active session, there will be no way for students to receive these required reviews, effectively blocking the graduation and upgrade process. This could happen due to:

  1. Malicious behavior by the principal

  2. Accidental removal of all teachers

  3. Governance disagreement leading to removal of teachers

This creates a situation where students have paid their school fees but cannot progress through the system as designed.

Impact

The impact of this issue is:

  1. School sessions can become deadlocked with no path to completion

  2. Students may not be able to receive required reviews

  3. The system cannot be upgraded once stuck in this state

  4. School fees paid by students are effectively locked

This is assessed as Low impact rather than Medium because:

  • It requires principal action (potentially malicious or severely negligent)

  • It does not directly lead to fund loss but rather operational issues

  • There are likely governance mechanisms outside the contract scope to resolve such situations

Tools Used

Manual code review

Recommendation

Add a safeguard to prevent removing the last teacher during an active session:

function removeTeacher(address _teacher) public onlyPrincipal {
if (_teacher == address(0)) {
revert HH__ZeroAddress();
}
​
if (!isTeacher[_teacher]) {
revert HH__TeacherDoesNotExist();
}
// Add check to prevent removing the last teacher during an active session
if (inSession && listOfTeachers.length == 1) {
revert("Cannot remove the last teacher during an active session");
}
​
uint256 teacherLength = listOfTeachers.length;
for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
break;
}
}
​
isTeacher[_teacher] = false;
​
emit TeacherRemoved(_teacher);
}

This ensures that at least one teacher remains available to give reviews during an active session, preventing the school system from entering a deadlocked state.

Updates

Lead Judging Commences

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

Support

FAQs

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