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

Session can start without teachers due to `LevelOne::startSession` missing contract behaviour checks

blackgrease

Summary

A session at Hawk High can be started without having any teachers employed which is outside of the expected contract logic. This logic check should be enforced by the contract for trust and enforcement of job roles.

Description

A school in any scenario should not be able to start a session without having any teachers in employment.

The Code Issue

This is the case in the Hawk High LevelOne contract where the startSession function can be successfully called by the Principal before any teachers are added due to missing checks on whether any teacher has been added.

The LevelOne::addTeacher function can only be called when a session has not yet started. Once a session has started, it cannot be called, Furthermore, once a session is started it cannot be stopped. In other words, this is a one-time mistake that is costly as there is no "undo".

The contract has a centralization issue where the Principal holds power over most of the sensitive functionality of the contract. While the Principal is assumed to be trusted, the contract should mitigate the centralization risks by enforcing rules to make sure the Principal cannot make mistakes and handles their job duties properly.

The Affected Code Area

Here is the affected function where there are no checks that the total number of teachers is not 0.

​
function startSession(uint256 _cutOffScore) public onlyPrincipal notYetInSession {//@audit: no check for the number of teachers
sessionEnd = block.timestamp + 4 weeks;
inSession = true;
cutOffScore = _cutOffScore;
​
emit SchoolInSession(block.timestamp, sessionEnd);
}

Impact

The severity of this logic flaw is Medium. While the impact is High as it involves

  1. Student loss of funds.

    1. Once enrolled, they cannot get their fees out i.e un-enroll.

    2. Teachers cannot be added once a session has started and therefore the deployed contract is a waste of expenses and funds.

  2. Broken contract functionality due to flawed logic and no checks enforcing behavior.

However, the likelihood of this is Low...on the assumption the Principal can execute their job duties properly.

Tools Used

  1. Manual Review

  2. Foundry for PoC

Proof-Of-Concept

Description

I have created a runnable PoC in Foundry to prove the validity of the issue:

  1. Enrolls 15 students into Hawk High

  2. Calls startSession as the Principal

  3. Asserts that the total number of Teachers is 0 and that the session state is true (ie started).

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 {LevelTwo} from "src/LevelTwo.sol";
import {DeployLevelOne} from "script/DeployLevelOne.s.sol";
import {MockUSDC} from "test/mocks/MockUSDC.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
contract DoSByStudentRemoval is Test {
DeployLevelOne deployBot;
LevelOne levelOneProxy;
LevelTwo levelTwoLogic;
MockUSDC usdc;
​
address proxyAddress;
address levelOneImplementationAddress;
​
//Roles
address principal;
address[] students;
​
//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();
}
​
modifier enrollStudents() {
//Student Creation & Enrolling fees
uint256 numberOfStudents = 16;
students = new address[](numberOfStudents);
​
for (uint256 x = 1; x < numberOfStudents; x++) {
students[x] = address(uint160(x));
​
vm.startPrank(students[x]);
usdc.mint(students[x], schoolFees);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
}
_;
}
​
function testStartSessionWithoutTeachers() public enrollStudents {
//Students have enrolled and a session is started.
vm.startPrank(principal);
levelOneProxy.startSession(5);
vm.stopPrank();
​
//Asserting there are no teachers
uint256 numberOfTeachers = levelOneProxy.getTotalTeachers();
uint256 numberOfStudents = levelOneProxy.getTotalStudents();
uint256 sessionEnd = levelOneProxy.sessionEnd();
uint256 currentTime = block.timestamp;
​
bool hasSessionStarted = levelOneProxy.getSessionStatus();
​
assertEq(numberOfTeachers, 0);
assertEq(hasSessionStarted,true);
​
//Logging
console2.log("Session has started");
console2.log("Current timestamp: ", currentTime);
console2.log("Session Ending in: ", sessionEnd);
console2.log("Has the session started? [true=yes,false=no]: ", hasSessionStarted);
console2.log("Number of teachers: ", numberOfTeachers);
console2.log("Number of students: ", numberOfStudents);
}
}
​

Mitigation

In order to mitigate this vulnerability, the startSession function should have a check to make sure the total number of teachers is not 0.

​
function startSession(uint256 _cutOffScore) public onlyPrincipal notYetInSession {
+ require(listOfTeachers.length != 0,"Cannot start session without teachers");
sessionEnd = block.timestamp + 4 weeks;
inSession = true;
cutOffScore = _cutOffScore;
​
emit SchoolInSession(block.timestamp, sessionEnd);
}
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

blackgrease Submitter
6 months ago
yeahchibyke Lead Judge
6 months ago
yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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