Summary
the event Graduated(address indexed LevelTwo)
is not emmited inthe graduateAndUpgrade()
function.
Vulnerability Details
there is a missing Event emmission in the graduateAndUpgrade()
function, this is a poor practice as an event should be emmitted after a change in storage.
poc
Look at the test below, this test fails because the emit is expected in the function but bot gotten.
function test_graduateAndUpgradeDoesNotEmitEvent_poc() public {
_teachersAdded();
_studentsEnrolled();
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
vm.expectEmit(true, false, false, false);
emit Graduated(levelTwoImplementationAddress);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
}
Tools Used
manual review
Recommendations
add the emit to the function as shown below
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
if (inSession == true) {
revert HH__AlreadyInSession();
}
if (listOfStudents.length == 0) {
revert HH__ZeroValue();
}
if (listOfTeachers.length == 0) {
revert HH__ZeroValue();
}
uint256 totalStudents = listOfStudents.length;
for (uint256 n = 0; n < totalStudents; n++) {
if (reviewCount[listOfStudents[n]] != 4) {
revert HH__CannotUpgradeUntilAllReviewsAreGiven();
}
}
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
_authorizeUpgrade(_levelTwo);
+ emit Graduated(_levelTwo);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}