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

Zero teacher possible during a session

0xbc000

Summary

In file LevelOne.sol function removeTeacher()

It didn't check if the current session in true and if the teacher is the last teacher within the session.


Vulnerability Details

In the removeTeacher() function, it missing checks if this is the last teacher available during the session is on.

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);
}

On the other side, there's a notYetInSession check for the addTeacher() function.

This is business logic issue.

function addTeacher(address _teacher) public onlyPrincipal notYetInSession {
if (_teacher == address(0)) {
revert HH__ZeroAddress();
}
​
if (isTeacher[_teacher]) {
revert HH__TeacherExists();
}
​
if (isStudent[_teacher]) {
revert HH__NotAllowed();
}
​
listOfTeachers.push(_teacher);
isTeacher[_teacher] = true;
​
emit TeacherAdded(_teacher);
}

Impact

There could be no teacher during a session is on


Proof of Concept

Add the following test into test/LeveOnelAndGraduateTest.t.sol

// In this test, it done the followings
// 1. Added 2 teachers and start the session
// 2. Remove all teachers
// 3. assert no teacher while session is on
function test_confirm_remove_teacher_during_session() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
levelOneProxy.startSession(70);
vm.stopPrank();
​
vm.startPrank(principal);
levelOneProxy.removeTeacher(alice);
levelOneProxy.removeTeacher(bob);
vm.stopPrank();
​
assert(levelOneProxy.getTotalTeachers() == 0);
assert(levelOneProxy.getSessionStatus() == true);
}

Tools Used

Manual review


Recommendations

Considering adding checks in removeTeacher() function

  • Add a notYetInSession modifier

or

  • Given a session in on, check if this is the last teacher before remove

Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 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.