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

Unbounded Loop in removeTeacher() Function Creates DoS Risk

khalidwalid

Description: The removeTeacher() function in the LevelOne contract contains an unbounded loop that iterates through the entire listOfTeachers array to find and remove a specific teacher. As the number of teachers grows, this operation will consume increasing amounts of gas, potentially exceeding block gas limits.

function removeTeacher(address _teacher) public onlyPrincipal {
// [...validation logic...]
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);
}

Impact: If the number of teachers becomes too large, the function will require more gas than the block gas limit allows, making it impossible to remove teachers. This would lock the principal into a state where they cannot manage the teaching staff, breaking a core contract function.

Proof of Concept:

  1. Assume the principal adds 1,000+ teachers to the system.

  2. When attempting to remove a teacher near the end of the array or one that doesn't exist, the loop must iterate through most or all teachers.

  3. Each iteration consumes gas for storage reads and comparisons.

  4. The transaction will fail with an "out of gas" error when the required gas exceeds the block limit (currently ~30M on Ethereum mainnet).

Recommended Mitigation: Replace the array-based storage of teachers with a mapping-based approach and a separate array for enumeration. Implement a mechanism to track the index of each teacher in the array:

mapping(address => uint256) private teacherIndices;
​
function removeTeacher(address _teacher) public onlyPrincipal {
// [...validation logic...]
uint256 indexToRemove = teacherIndices[_teacher];
address lastTeacher = listOfTeachers[listOfTeachers.length - 1];
listOfTeachers[indexToRemove] = lastTeacher;
teacherIndices[lastTeacher] = indexToRemove;
listOfTeachers.pop();
delete teacherIndices[_teacher];
isTeacher[_teacher] = false;
emit TeacherRemoved(_teacher);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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