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

DoS Risk in Teacher Payment Distribution Loop

khalidwalid

Description: The graduateAndUpgrade() function in the LevelOne contract contains an unbounded loop that iterates through all teachers to distribute payments. As the number of teachers increases, this function will consume more gas and can eventually exceed block gas limits, preventing successful execution of the graduation and upgrade process.

Code Snippet:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
​
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}

Impact: If the number of teachers grows too large, the contract will be unable to complete the graduation and upgrade process. This prevents the contract from upgrading to a new implementation and distributing payments to teachers and the principal, effectively locking funds in the contract and halting the progression of the school system.

Proof of Concept:

  1. The principal adds a large number of teachers (100+).

  2. When attempting to call graduateAndUpgrade(), each teacher receives a payment via safeTransfer().

  3. Each transfer consumes significant gas (~21,000 base + ~5,000 for the ERC20 transfer).

  4. With enough teachers, the total gas required will exceed the block gas limit (~30M on Ethereum).

  5. The transaction will fail with an "out of gas" error, preventing the upgrade.

Recommended Mitigation: Implement a batch processing pattern to distribute payments across multiple transactions:

// Add state variables to track graduation progress
bool private isGraduating;
uint256 private graduationProcessedTeachers;
address private pendingUpgrade;
​
function startGraduation(address _levelTwo) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
isGraduating = true;
graduationProcessedTeachers = 0;
pendingUpgrade = _levelTwo;
// Calculate payments
payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
}
​
function processBatchPayments(uint256 batchSize) public onlyPrincipal {
require(isGraduating, "Graduation not started");
uint256 end = Math.min(graduationProcessedTeachers + batchSize, listOfTeachers.length);
for (uint256 i = graduationProcessedTeachers; i < end; i++) {
usdc.safeTransfer(listOfTeachers[i], payPerTeacher);
}
graduationProcessedTeachers = end;
// Complete the process if all teachers are paid
if (graduationProcessedTeachers == listOfTeachers.length) {
usdc.safeTransfer(principal, principalPay);
_authorizeUpgrade(pendingUpgrade);
isGraduating = false;
}
}
Updates

Lead Judging Commences

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

inefficient teacher payment system

Due to the use of a push system as regards payment of teacher wages, there is a risk of possible DoS as gas costs increase in direct proportion to size of teachers list.

Support

FAQs

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