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

Lack of notYetInSession Modifier in removeTeacher Function

sekkiat

Summary

The removeTeacher function in the contract lacks a notYetInSession modifier, which is present in the addTeacher function. This oversight allows the principal to remove teachers even after the session has started, potentially causing unfair reward distribution during graduateAndUpgrade processing.

Vulnerability Details

The addTeacher function includes the notYetInSession modifier to prevent teachers from being added once the session is in progress (to ensure fairness and reward accuracy), it does not apply the same constraint on removeTeacher function. This creates an imbalance, where a teacher who has already participated in the session can be removed at the last minute, making them ineligible for compensation fromgraduateAndUpgrade

//missing notYetInSession modifier
function removeTeacher(address _teacher) public onlyPrincipal {
if (_teacher == address(0)) {
revert HH__ZeroAddress();
}
​
if (!isTeacher[_teacher]) {
revert HH__TeacherDoesNotExist();
}
​
uint256 teacherLength = listOfTeachers.length;
for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
break;
}
}
​
isTeacher[_teacher] = false;
​
emit TeacherRemoved(_teacher);
}

POC

function test_remove_teacher_and_distribute_funds() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
//Remove teacher before distribute rewards
vm.prank(principal);
levelOneProxy.removeTeacher(alice);
​
assert(levelOneProxy.isTeacher(alice) == false);
assert(levelOneProxy.getTotalTeachers() == 1);
console2.log("Removed Teacher Alice");
console2.log("Teacher Alice USDC Amount (Before)", usdc.balanceOf(alice));
console2.log("Teacher Alice USDC Amount (Before)", usdc.balanceOf(bob));
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
//Teacher Alice get nothing
console2.log("Teacher Alice USDC Amount (After)", usdc.balanceOf(alice));
console2.log("Teacher Alice USDC Amount (After)", usdc.balanceOf(bob));
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
console2.log(levelTwoProxy.bursary());
console2.log(levelTwoProxy.getTotalStudents());
}

Impact

A malicious principal could exploit this gap by removing a teacher just before the session ends, preventing them from being included in reward or upgrade logic tied to graduateAndUpgrade. This can result in loss of earned compensation for that teacher, undermining trust in the system and potentially leading to disputes or abuse.

Tools Used

Manual Review

Recommendations

Add the notYetInSession modifier to the removeTeacher function to mirror the protection used in addTeacher.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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