Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Unbounded Loop in expel() Function May Cause Denial of Service

khalidwalid

Description: The expel() function in the LevelOne contract contains an unbounded loop that iterates through the entire listOfStudents array to find and remove a specific student. As the school grows and the number of enrolled students increases, this operation will consume more gas, potentially exceeding the block gas limit and making it impossible to expel students.

Code Snippet:

function expel(address _student) public onlyPrincipal {
if (inSession == false) {
revert();
}
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;
​
emit Expelled(_student);
}

Impact: If the number of enrolled students becomes too large, the principal will be unable to expel students due to gas limitations. This breaks a critical administrative function of the contract, potentially preventing the principal from enforcing school discipline or removing problematic students.

Proof of Concept:

  1. Assume the school enrollment grows to several hundred or thousand students.

  2. The principal attempts to expel a student who is positioned near the end of the array.

  3. The loop must iterate through most or all students, consuming gas for each iteration.

  4. Once the number of students is large enough, the transaction will fail with an "out of gas" error.

Recommended Mitigation: Implement a more gas-efficient method for tracking and removing students by using a mapping to track each student's index in the array:

mapping(address => uint256) private studentIndices;
​
function enroll() external notYetInSession {
// Existing validation...
studentIndices[msg.sender] = listOfStudents.length;
listOfStudents.push(msg.sender);
isStudent[msg.sender] = true;
studentScore[msg.sender] = 100;
bursary += schoolFees;
​
emit Enrolled(msg.sender);
}
​
function expel(address _student) public onlyPrincipal {
// Existing validation...
uint256 indexToRemove = studentIndices[_student];
uint256 lastIndex = listOfStudents.length - 1;
if (indexToRemove != lastIndex) {
address lastStudent = listOfStudents[lastIndex];
listOfStudents[indexToRemove] = lastStudent;
studentIndices[lastStudent] = indexToRemove;
}
listOfStudents.pop();
delete studentIndices[_student];
isStudent[_student] = false;
emit Expelled(_student);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
6 months ago
yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

possible DoS when expelling students

Unbounded loops in student lists could result in high gas usage when trying to expel a students when students are plenty. This could result in a possible DoS

Support

FAQs

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