There is no check that the school session isn't over before graduating the students and upgrading the contract. This means that graduateAndUpgrade
can be called instantly after starting a school session.
This PoC only works if the vulnerablities affecting the storage layout and invalid implementation are fixed, otherwise we can't upgrade the contract and therefore cannot check it's state.
function test_system_can_upgrade_without_session_end() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(principal);
levelOneProxy.startSession(70);
vm.stopPrank();
vm.warp(block.timestamp + 1 weeks);
console2.log("Current block timestamp: ", block.timestamp);
console2.log("Current session end: ", levelOneProxy.sessionEnd());
vm.startPrank(principal);
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
assert(levelTwoProxy.getTotalStudents() == 1);
console2.log("Total students in new contract: ", levelTwoProxy.getTotalStudents());
## Impact
This missing check allows students to graduate without recieving reviews and teachers to be paid without fulfilling their obligations, this defeats the core functionality of the protocol.
## Tools Used
Manual review.
## Recommendations
Check that `block.timestamp > sessionEnd`.
```diff
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ require(block.timestamp > sessionEnd, "School session has not concluded");
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
_authorizeUpgrade(_levelTwo);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}