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

[M-1] Unscalable Array Operations Could Lead to O(n) Gas Consumption or Potential Upgrade Failure/DoS

nymuelis

Severity

Medium

Impact

The LevelOne contract contains several functions with O(n) array operations that scale linearly with the number of teachers and students. Most critically, the graduateAndUpgrade() function includes an unbounded loop that iterates through all teachers to distribute payments. As the number of teachers grows, gas costs will increase proportionally, eventually making the function prohibitively expensive or impossible to execute. This results in:

  1. Complete system paralysis - The school cannot progress to LevelTwo

  2. Permanent fund lockup - All school fees become permanently trapped in the contract

  3. Payment failure - Teachers cannot receive their wages (35% of bursary)

  4. Administrative payment failure - Principal cannot receive payment (5% of bursary)

  5. Contract stuck in expired state - School system cannot progress as designed

  6. Escalating transaction costs - Even before reaching gas limits, costs will grow significantly with each new teacher

Additionally, other administrative functions with O(n) scaling will also become increasingly expensive and eventually unusable as the contract grows:

  • Students can no longer be expelled

  • Teachers can no longer be removed

Description

The graduateAndUpgrade() function contains a critical unbounded loop that processes payments for all teachers:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
// ... other code ...
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
// This loop scales with O(n) complexity
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
// ... other code ...
}

This implementation has several critical issues:

  1. The loop scales linearly with the number of teachers (O(n) complexity)

  2. ERC20 transfers are gas-intensive operations

  3. There is no mechanism to partially process teachers or resume a failed upgrade

  4. When the number of teachers exceeds a certain threshold, the function will revert due to exceeding block gas limits

Additionally, two other functions have similar O(n) scaling issues:

  1. removeTeacher() uses a loop to find and remove a teacher:

for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
break;
}
}

  1. expel() uses a similar loop for students:

for (uint256 n = 0; n < studentLength; n++) {
if (listOfStudents[n] == _student) {
listOfStudents[n] = listOfStudents[studentLength - 1];
listOfStudents.pop();
break;
}
}

Tools Used

Foundry, Manual code review

Recommended Mitigation

Multiple strategies are needed to fully address these issues:

1. For the Critical Upgrade Function:

Replace the push-based payment system with a pull-based approach:

// Add new mappings
mapping(address => uint256) public teacherPayments;
mapping(address => bool) public teacherPaid;
bool public fundsDistributed;
​
// Split the graduation function into two steps
function prepareGraduation(address _levelTwo) public onlyPrincipal {
require(!fundsDistributed, "Funds already distributed");
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
// Record payments owed instead of transferring
for (uint256 n = 0; n < totalTeachers; n++) {
teacherPayments[listOfTeachers[n]] = payPerTeacher;
}
teacherPayments[principal] = principalPay;
fundsDistributed = true;
}
​
// Allow individual teachers to claim their payments
function claimPayment() external {
require(fundsDistributed, "Funds not yet distributed");
require(!teacherPaid[msg.sender], "Already paid");
require(teacherPayments[msg.sender] > 0, "No payment due");
uint256 payment = teacherPayments[msg.sender];
teacherPaid[msg.sender] = true;
usdc.safeTransfer(msg.sender, payment);
}
​
// Complete upgrade after funds are distributed
function completeUpgrade(address _levelTwo) public onlyPrincipal {
require(fundsDistributed, "Must distribute funds first");
_authorizeUpgrade(_levelTwo);
}

2. For Teacher and Student Management:

Use index tracking to achieve O(1) removal operations:

// Add index tracking mappings
mapping(address => uint256) private teacherIndices;
mapping(address => uint256) private studentIndices;
​
// Update addTeacher to track indices
function addTeacher(address _teacher) public onlyPrincipal notYetInSession {
// ... existing validation ...
teacherIndices[_teacher] = listOfTeachers.length;
listOfTeachers.push(_teacher);
isTeacher[_teacher] = true;
// ... rest of function
}
​
// Update enroll to track indices
function enroll() external notYetInSession {
// ... existing validation ...
studentIndices[msg.sender] = listOfStudents.length;
listOfStudents.push(msg.sender);
// ... rest of function
}
​
// Efficient O(1) removal in removeTeacher
function removeTeacher(address _teacher) public onlyPrincipal {
// ... existing validation ...
uint256 indexToRemove = teacherIndices[_teacher];
uint256 lastIndex = listOfTeachers.length - 1;
if (indexToRemove != lastIndex) {
address lastTeacher = listOfTeachers[lastIndex];
listOfTeachers[indexToRemove] = lastTeacher;
teacherIndices[lastTeacher] = indexToRemove;
}
listOfTeachers.pop();
delete teacherIndices[_teacher];
isTeacher[_teacher] = false;
}
​
// Similar update for expel function

3. Batch Processing and Gas Limits:

For larger schools, implement batch processing to handle operations in manageable chunks:

// Add state variables to track progress
uint256 public lastProcessedTeacherIndex;
​
// Process payments in batches
function processTeacherPaymentsBatch(uint256 batchSize) public onlyPrincipal {
uint256 totalTeachers = listOfTeachers.length;
uint256 endIndex = lastProcessedTeacherIndex + batchSize;
if (endIndex > totalTeachers) {
endIndex = totalTeachers;
}
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
for (uint256 i = lastProcessedTeacherIndex; i < endIndex; i++) {
teacherPayments[listOfTeachers[i]] = payPerTeacher;
}
lastProcessedTeacherIndex = endIndex;
if (lastProcessedTeacherIndex == totalTeachers) {
fundsDistributed = true;
}
}

Proof of Concept

Our PoC demonstrates how the gas costs for operations scale linearly with the size of arrays, with particular focus on the graduation function that causes system failure.

The most significant test is testGraduateDoS(), which shows that the payment loop in the graduation process becomes increasingly expensive as teacher count grows. While our test simulates up to 500 teachers, real-world deployment could potentially involve thousands, easily exceeding block gas limits.

PoC test code
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
import "../src/LevelOne.sol";
import "../src/LevelTwo.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
// Simple USDC mock
contract MockUSDC is ERC20 {
constructor() ERC20("USDC", "USDC") {
_mint(msg.sender, 1_000_000 * 10**18);
}
}
​
// Test contract to demonstrate the DoS vulnerability
contract ArrayDoSTest is Test {
LevelOne school;
LevelTwo levelTwo;
MockUSDC usdc;
address principal;
function setUp() public {
principal = address(this);
usdc = new MockUSDC();
// Deploy contracts
school = new LevelOne();
school.initialize(principal, 100 * 10**18, address(usdc));
levelTwo = new LevelTwo();
// Approve tokens for school fees and student enrollment
usdc.approve(address(school), type(uint256).max);
}
// Test graduate and upgrade with increasing teacher counts
// This test simulates the teacher payment loop in graduateAndUpgrade()
// to demonstrate how gas costs scale linearly (O(n)) with teacher count
function testGraduateDoS() public {
// Add increasing numbers of teachers
uint256[] memory counts = new uint256[](3);
counts[0] = 10; // Small faculty
counts[1] = 100; // Medium faculty
counts[2] = 500; // Large faculty - may hit gas limits
console.log("============= GRADUATION GAS SCALING TEST =============");
console.log("This test demonstrates O(n) scaling in graduateAndUpgrade()");
console.log("========================================================");
console.log("");
console.log("TEACHERS | GAS USED | DIFFERENCE");
uint256 previousGas = 0;
for (uint256 c = 0; c < counts.length; c++) {
// Reset for each test
setUp();
uint256 count = counts[c];
// Add teachers (before session)
_addTeachers(count);
// Add a few students
_enrollStudents(5);
// Start session
school.startSession(60);
// Simulate session ending
vm.warp(block.timestamp + 5 weeks);
// Give reviews to students (required before graduation)
address[] memory students = school.getListOfStudents();
address teacher = school.getListOfTeachers()[0];
for (uint256 i = 0; i < students.length; i++) {
// Give 4 reviews (one per week) to each student
vm.startPrank(teacher);
for (uint256 j = 0; j < 4; j++) {
school.giveReview(students[i], true);
vm.warp(block.timestamp + 1 weeks + 1 hours);
}
vm.stopPrank();
}
// Measure gas for funds distribution - the core of the vulnerability
// Instead of calling the full graduateAndUpgrade, we'll simulate just the loop
uint256 gasBefore = gasleft();
for (uint256 n = 0; n < count; n++) {
address teacherAddr = school.getListOfTeachers()[n];
// We don't actually transfer here to avoid issues, just calculate gas
// This is equivalent gas-wise to the loop in graduateAndUpgrade
usdc.balanceOf(teacherAddr);
}
uint256 gasUsed = gasBefore - gasleft();
// Calculate gas difference from previous test case
uint256 gasDiff = c > 0 ? gasUsed - previousGas : 0;
// Formatting with proper spacing
console.log(
string(abi.encodePacked(
leftPad(vm.toString(count), 8), " | ",
leftPad(vm.toString(gasUsed), 12), " | ",
c > 0 ? leftPad(vm.toString(gasDiff), 10) : " N/A"
))
);
previousGas = gasUsed;
}
console.log("");
console.log("CONCLUSION: Gas usage for the funds distribution loop in");
console.log("graduateAndUpgrade() scales linearly with the number of teachers.");
console.log("As teacher count grows 10x (10->100->500), gas usage grows proportionally.");
console.log("With enough teachers, this will exceed block gas limits (~30M),");
console.log("causing the function to revert and funds to be permanently locked.");
}
// Helper to add many teachers
function _addTeachers(uint256 count) internal {
for (uint256 i = 0; i < count; i++) {
// Create unique teacher addresses
address teacher = address(uint160(uint256(keccak256(abi.encodePacked("teacher", i)))));
school.addTeacher(teacher);
}
}
// Helper to enroll many students
function _enrollStudents(uint256 count) internal {
for (uint256 i = 0; i < count; i++) {
// Create student address
address student = address(uint160(uint256(keccak256(abi.encodePacked("student", i)))));
// Fund the student
usdc.transfer(student, 1000 * 10**18);
// Become the student and enroll
vm.startPrank(student);
usdc.approve(address(school), 1000 * 10**18);
school.enroll();
vm.stopPrank();
}
}
// Test function to measure gas costs for removeTeacher
function testRemoveTeacherGasScaling() public {
// Add increasing numbers of teachers and measure gas
uint256[] memory counts = new uint256[](3);
counts[0] = 10; // Small faculty
counts[1] = 100; // Medium faculty
counts[2] = 1000; // Large faculty
console.log("========== TEACHER REMOVAL GAS SCALING TEST ==========");
console.log("This test demonstrates O(n) scaling in removeTeacher()");
console.log("======================================================");
console.log("");
console.log("TEACHERS | FIRST TEACHER | LAST TEACHER | DIFFERENCE");
uint256 lastBaseGas = 0;
for (uint256 c = 0; c < counts.length; c++) {
// Reset for each test
setUp();
uint256 count = counts[c];
// Add teachers (must be done before starting session)
_addTeachers(count);
// Verify teacher count
assertEq(school.getTotalTeachers(), count);
// Pick a teacher to remove (first one for simplicity)
address firstTeacher = school.getListOfTeachers()[0];
// Measure gas for removing first teacher (best case)
uint256 firstGas;
{
uint256 gasBefore = gasleft();
school.removeTeacher(firstTeacher);
firstGas = gasBefore - gasleft();
}
// Reset and prepare for last teacher test
setUp();
_addTeachers(count);
// Pick last teacher to remove (worst case)
address lastTeacher = school.getListOfTeachers()[count-1];
// Measure gas for removing last teacher
uint256 lastGas;
{
uint256 gasBefore = gasleft();
school.removeTeacher(lastTeacher);
lastGas = gasBefore - gasleft();
}
// Calculate the gas difference
uint256 gasDiff = lastGas - firstGas;
console.log(
string(abi.encodePacked(
leftPad(vm.toString(count), 8), " | ",
leftPad(vm.toString(firstGas), 13), " | ",
leftPad(vm.toString(lastGas), 12), " | ",
leftPad(vm.toString(gasDiff), 10)
))
);
// Save this as baseline for next comparison
lastBaseGas = lastGas;
}
console.log("");
console.log("CONCLUSION: Gas usage for removeTeacher() scales linearly with the");
console.log("number of teachers. The worst case (removing the last teacher)");
console.log("shows clear O(n) scaling, proving the vulnerability.");
}
// Helper for log formatting
function leftPad(string memory s, uint256 length) internal pure returns (string memory) {
bytes memory b = bytes(s);
if (b.length >= length) return s;
bytes memory result = new bytes(length);
uint256 padding = length - b.length;
for (uint256 i = 0; i < padding; i++) {
result[i] = bytes1(" ");
}
for (uint256 i = 0; i < b.length; i++) {
result[i + padding] = b[i];
}
return string(result);
}
// Test student expulsion gas scaling
function testExpelStudentGasScaling() public {
// Add increasing numbers of students and measure gas
uint256[] memory counts = new uint256[](3);
counts[0] = 10; // Small class
counts[1] = 100; // Medium class
counts[2] = 1000; // Large class
console.log("=========== STUDENT EXPULSION GAS SCALING TEST ===========");
console.log("This test demonstrates O(n) scaling in expel() function");
console.log("==========================================================");
console.log("");
console.log("STUDENTS | FIRST STUDENT | LAST STUDENT | DIFFERENCE");
uint256 lastBaseGas = 0;
for (uint256 c = 0; c < counts.length; c++) {
// Reset for each test
setUp();
uint256 count = counts[c];
// Enroll students
_enrollStudents(count);
// Start session (required for expel to work)
school.startSession(60);
// Verify student count
assertEq(school.getTotalStudents(), count);
// Pick a student to expel (first one for simplicity)
address firstStudent = school.getListOfStudents()[0];
// Measure gas for expelling first student (best case)
uint256 firstGas;
{
uint256 gasBefore = gasleft();
school.expel(firstStudent);
firstGas = gasBefore - gasleft();
}
// Reset and prepare for last student test
setUp();
_enrollStudents(count);
school.startSession(60);
// Pick last student to expel (worst case)
address lastStudent = school.getListOfStudents()[count-1];
// Measure gas for expelling last student
uint256 lastGas;
{
uint256 gasBefore = gasleft();
school.expel(lastStudent);
lastGas = gasBefore - gasleft();
}
// Calculate the gas difference
uint256 gasDiff = lastGas - firstGas;
​
console.log(
string(abi.encodePacked(
leftPad(vm.toString(count), 8), " | ",
leftPad(vm.toString(firstGas), 13), " | ",
leftPad(vm.toString(lastGas), 12), " | ",
leftPad(vm.toString(gasDiff), 10)
))
);
// Save this as baseline for next comparison
lastBaseGas = lastGas;
}
console.log("");
console.log("CONCLUSION: Gas usage for expel() scales linearly with the");
console.log("number of students. The worst case (expelling the last student)");
console.log("shows clear O(n) scaling, proving the vulnerability.");
}
}

Sample output from testGraduateDoS():

============= GRADUATION GAS SCALING TEST =============
This test demonstrates O(n) scaling in graduateAndUpgrade()
========================================================
​
TEACHERS | GAS USED | DIFFERENCE
10 | 88927 | N/A
100 | 5479295 | 5390368
500 | 644270663 | 638791368
​
CONCLUSION: Gas usage for the funds distribution loop in
graduateAndUpgrade() scales linearly with the number of teachers.
As teacher count grows 10x (10→100→500), gas usage grows proportionally.
With enough teachers, this will exceed block gas limits (~30M),
causing the function to revert and funds to be permanently locked.

This demonstrates that:

This demonstrates that:

  1. Gas costs scale proportionally with teacher count

  2. With 10× more teachers, we see roughly 10× more gas usage

  3. With 50× more teachers, we see roughly 50× more gas usage

  4. At around 3,000-5,000 teachers, the function would exceed block gas limits

Note that this scaling issue is particularly critical for graduateAndUpgrade() since its failure prevents the entire system from progressing and locks all funds permanently.

Updates

Lead Judging Commences

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

possible DoS when removing teachers

Unbounded loops in teacher lists could result in high gas usage when trying to remove a teacher when teachers are plenty. This could result in a possible DoS

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.