Hawk High

First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

System can upgrade before the school session ends.

Summary

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.

Vulnerability Details

We never check that current block.timestamp is greater than sessionEnd. This means that the protocol can upgrade before the school session ends.

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.

PoC:

function test_system_can_upgrade_without_session_end() public {
// Adds 2 teachers to the school
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
// Enroll a student
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
// Start school session
vm.startPrank(principal);
levelOneProxy.startSession(70);
vm.stopPrank();
// Check that sessionEnd is greater than current timestamp
vm.warp(block.timestamp + 1 weeks);
console2.log("Current block timestamp: ", block.timestamp);
console2.log("Current session end: ", levelOneProxy.sessionEnd());
// Principal calls graduateAndUpgrade
vm.startPrank(principal);
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// Principal performs actual upgrade
// This passes after fixing wrong storage layout and invalid implementation.
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
// Check that the student is in the new contract
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);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

can graduate without session end

`graduateAndUpgrade()` can be called successfully even when the school session has not ended

Support

FAQs

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