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

Potential Denial of Service via gas consumption on `LevelOne::expel` function due to unbounded for loop

blackgrease

Summary

In Hawk High, due to not having a limit on the maximum number of students, the LevelOne::expel function is a potential vector for Denial of Service either running out of gas or becoming infeasible to remove a student due to gas cost.

Description

The Hawk High LevelOne contract is potentially vulnerable to a Denial of Service attack via an unbounded array in the LevelOne::expel function.

The Code Issue

The LevelOne:expel function is used by the Principal to remove a student from Hawk High. The function iterates over a for loop that is unbounded. It is unbounded because there is no set maximum limit to the number of students that can enroll at Hawk High.

Therefore, for a large number of enrolled students, it becomes gas expensive for the Principal to remove a student, especially if the student is indexed at the end of the listOfStudents array. If there are many students enrolled, the function may

  1. consume all gas when run causing a Denial of service,

  2. become unfeasible to expel a student due to high gas cost

In either case, the intended functionality is broken.

Affected Code Area

​
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; //@audit: if this is large, potential DoS vector
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

The severity of this bug is Medium as there is no impact to any funds, directly or indirectly. However, it breaks the expected functionality of being able to expel a student in a feasible manner.

When there are many students enrolled, the function may:

  1. consume all gas when run causing a Denial of service, failing to expel the student

  2. become unfeasible to expel a student as it is expensive

Tools Used

  1. Manual Review

  2. Foundry

Proof of Concept

Description

I have created a runnable Proof-of-Concept that deploys two instances of Hawk High - Level One. One instance has a few students enrolled (30), while the second instance has a larger number of enrolled students (3000). The gas expense of expelling a student from each are compared to show that it is more expensive to expel a student when more are enrolled.

Note: if multiple students are to be expelled, this is an even more gas expensive operation.

Running: forge test --mt testPotentialDoSonExpelingStudents -vvv

Code

​
//SPDX-License-Identifier: MIT
​
pragma solidity 0.8.26;
​
import {Test, console2} from "forge-std/Test.sol";
import {LevelOne} from "src/LevelOne.sol";
import {DeployLevelOne} from "script/DeployLevelOne.s.sol";
import {MockUSDC} from "test/mocks/MockUSDC.sol";
​
​
contract DoSByStudentRemoval is Test {
DeployLevelOne deployBot;
DeployLevelOne deployBot_2;
LevelOne levelOneProxy;
LevelOne levelOneProxy_2;
MockUSDC usdc;
MockUSDC usdc2;
​
address proxyAddress;
address proxyAddress_2;
address levelOneImplementationAddress;
​
//Roles
address principal;
address principal_2;
address[] public studentsFew;
address[] public studentsMany;
​
//Misc
uint256 schoolFees;
uint256 schoolFees_2;
​
function setUp() public {
//First Instance
deployBot = new DeployLevelOne();
proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
​
usdc = deployBot.getUSDC(); //used in both instances
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
//Second Instance
deployBot_2 = new DeployLevelOne();
proxyAddress_2 = deployBot_2.deployLevelOne();
levelOneProxy_2 = LevelOne(proxyAddress_2);
​
usdc2 = deployBot_2.getUSDC();
principal_2 = deployBot_2.principal();
schoolFees_2 = deployBot_2.getSchoolFees();
​
}
​
​
modifier startSession() {
vm.prank(principal);
levelOneProxy.startSession(5);
_;
}
​
modifier startSession2() {
vm.prank(principal_2);
levelOneProxy_2.startSession(5);
_;
}
​
modifier addFewStudents() {
//Student Creation & Enrolling
uint256 numberOfStudents = 30;
studentsFew = new address[](numberOfStudents);
​
for (uint256 x = 1; x < numberOfStudents; x++) {
studentsFew[x] = address(uint160(x));
​
vm.startPrank(studentsFew[x]);
usdc.mint(studentsFew[x], schoolFees);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
}
_;
}
​
​
modifier addManyStudents() {//Using second deployment of school for comparison
//Student Creation & Enrolling
uint256 numberOfStudents = 3000;
studentsMany = new address[](numberOfStudents);
​
for (uint256 x = 1; x < numberOfStudents; x++) {
studentsMany[x] = address(uint160(x));
​
vm.startPrank(studentsMany[x]);
usdc2.mint(studentsMany[x], schoolFees + 2);
usdc2.approve(address(levelOneProxy_2), schoolFees);
levelOneProxy_2.enroll();
vm.stopPrank();
}
_;
}
​
function GasConsumptionOnFewStudents() internal addFewStudents startSession returns (uint256) {
address studentToExpelFromFew = studentsFew[studentsFew.length - 1]; //the last index in the array making the loop run until the end
uint256 startingGasExpelFromFewStudents = gasleft();
​
vm.prank(principal);
levelOneProxy.expel(studentToExpelFromFew);
​
uint256 remainingGasExpelFromFewStudents = gasleft();
uint256 gasConsumptionExpelFromFewStudents = startingGasExpelFromFewStudents - remainingGasExpelFromFewStudents;
​
return gasConsumptionExpelFromFewStudents;
}
​
function GasConsumptionOnManyStudents() internal addManyStudents startSession2 returns (uint256) {
address studentToExpelFromMany = studentsMany[studentsMany.length - 1]; //the last index in the array making the loop run until the end
uint256 startingGas = gasleft();
​
vm.prank(principal_2);
levelOneProxy_2.expel(studentToExpelFromMany);
​
uint256 remainingGas = gasleft();
uint256 gasConsumption = startingGas - remainingGas;
​
return gasConsumption;
}
​
function testPotentialDoSonExpelingStudents() public {
​
uint256 gasConsumptionFewStudents = GasConsumptionOnFewStudents();
uint256 gasConsumptionManyStudents = GasConsumptionOnManyStudents();
​
uint256 gasDifference = gasConsumptionManyStudents - gasConsumptionFewStudents;
​
uint256 numberOfStudents = levelOneProxy.getTotalStudents();
uint256 numberOfStudents_2 = levelOneProxy_2.getTotalStudents();
​
// //Logging
console2.log("Number of students in first deployment: ", numberOfStudents);
console2.log("Number of students in second deployment: ", numberOfStudents_2);
console2.log("Gas used for few students: ", gasConsumptionFewStudents);
console2.log("Gas used for many students: ", gasConsumptionManyStudents);
console2.log("Gas Difference: ", gasDifference);
}
}
​
​

Mitigation

A recommended mitigation to this, is to set a reasonable maximum limit to the number of enrolled students.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 29 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

blackgrease Submitter
27 days ago
yeahchibyke Lead Judge 26 days 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.