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

Teachers array not capped => Potential DoS

mihaihng

Summary

This loop in graduateAndUpgrade calls safeTransfer on the list of teachers:

for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}

safeTransfer is an external call that consumes gas.

Vulnerability Details

In the case where listOfTeachers.length is very large the total gas needed may exceed the block gas limit, bringing the function in a point where it can't be executed or finish its execution.

Impact

The graduateAndUpgrade function can not be executed => DoS

The upgrade process is compromised.

The contract loses its main functionality.

Tools Used

Manual Code Review

Recommendation

Add a constant for MAX_TEACHERS.

uint256 public constant MAX_TEACHERS = 10;

Add a new conditional in addTeacher to check if the teachers array is always within boundries.

Add a new error for this check:


error HH__MaxNumberOfTeachersReached(uint256); // New error
​
function addTeacher(address _teacher) public onlyPrincipal notYetInSession {
if (_teacher == address(0)) {
revert HH__ZeroAddress();
}
​
if (isTeacher[_teacher]) {
revert HH__TeacherExists();
}
​
if (isStudent[_teacher]) {
revert HH__NotAllowed();
}
​
if (listOfTeachers.length > MAX_TEACHERS) { // New check for MAX number of teachers
revert HH__MaxNumberOfTeachersReached(MAX_TEACHERS);
}
​
listOfTeachers.push(_teacher);
isTeacher[_teacher] = true;
​
emit TeacherAdded(_teacher);
}

This will prevent the array to grow beyound limits.



Other solutions can be batching the payments or a pull-based system where the teachers withdraw their wages.


Updates

Lead Judging Commences

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

Support

FAQs

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