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

Unprotected Array Removals in Teacher/Student Management

0xki

Summary

The removeTeacher() and expel() functions in LevelOne.sol use a swap-and-pop pattern for array element removal that only removes the first occurrence of an address. If duplicate entries exist in the arrays (through storage manipulation or other vulnerabilities), the functions will leave "ghost entries" while updating the mapping to indicate removal, creating an inconsistent contract state.

Vulnerability Details

In removeTeacher() (lines 229-238):

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;
​

Similarly in expel() (lines 255-264):

for (uint256 n = 0; n < studentLength; n++) {
if (listOfStudents[n] == _student) {
listOfStudents[n] = listOfStudents[studentLength - 1];
listOfStudents.pop();
break; // Exits after first match
}
}
isStudent[_student] = false;
​

The break statement causes the loop to exit after finding the first occurrence, leaving any duplicate entries in the array while marking the address as removed in the mapping.

Impact

  • State Inconsistency: Arrays may contain addresses marked as non-teachers/non-students in mappings

  • Broken Access Control: Functions relying on array iteration may grant access to removed users

  • UI/Integration Issues: Off-chain systems relying on array data will show incorrect information

  • Potential Reentrancy Vectors: Ghost entries could be exploited in complex attack scenarios

Tools Used

Manual code review

Foundry forge

Recommendations


Replace the current removal logic with a comprehensive approach that removes all instances:

function removeTeacher(address _teacher) public onlyPrincipal {
// ... existing checks ...
uint256 initialLength = listOfTeachers.length;
uint256 i = 0;
while (i < listOfTeachers.length) {
if (listOfTeachers[i] == _teacher) {
listOfTeachers[i] = listOfTeachers[listOfTeachers.length - 1];
listOfTeachers.pop();
} else {
i++;
}
}
if (initialLength == listOfTeachers.length) {
revert HH__TeacherDoesNotExist();
}
isTeacher[_teacher] = false;
emit TeacherRemoved(_teacher);
}
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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