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

School Session Can Be Prematurely Ended Before Minimum Duration, Breaking The Invariant

geekybot

Summary

The graduateAndUpgrade function in the LevelOne contract allows the principal to end a school session at any time without enforcing a minimum duration. This violates the intended functionality where a school session should last at least 4 weeks, potentially allowing malicious principals to collect fees and end sessions prematurely. Which is a causing Invariant mentioned in the documentation

Vulnerability Details

When starting a session via the startSession function, the contract sets sessionEnd to 4 weeks in the future:

function startSession(uint256 _cutOffScore) external onlyPrincipal {
if (inSession) {
revert HH__SessionOngoing();
}
inSession = true;
sessionEnd = block.timestamp + 4 weeks;
cutOffScore = _cutOffScore;
​
emit SessionStarted(sessionEnd);
}

However, the graduateAndUpgrade function has no checks to ensure that the 4-week period has actually elapsed:

function graduateAndUpgrade(
address _levelTwo,
bytes memory
) public onlyPrincipal {
// q does it follow CEI
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
​
// q is this correct calculation for payPerTeacher?
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
// q what happens if a teacher contract is unable to receive USDC?
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}

This allows the principal to end the session immediately after starting it, without waiting for the 4-week duration to complete.

Impact

  • Students pay school fees for a complete session but might receive a shortened educational period

  • The principal can collect fees, end the session prematurely, and potentially start a new session to collect fees again

  • Teachers have less time to conduct reviews and evaluate students

  • Violates the contract's own declared session duration of 4 weeks

Tools Used

  • Manual code review

  • Custom test case

Proof of Concept

Add the following test to demonstrate the issue:

function test_premature_graduation() public schoolInSession {
// Setup
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
// Record initial session state
uint256 sessionStartTime = block.timestamp;
uint256 expectedSessionEnd = levelOneProxy.sessionEnd();
uint256 expectedSessionDuration = expectedSessionEnd - sessionStartTime;
// Log information for clarity
console2.log("Session start time:", sessionStartTime);
console2.log("Expected session end time:", expectedSessionEnd);
console2.log("Expected session duration (seconds):", expectedSessionDuration);
console2.log("Expected session duration (days):", expectedSessionDuration / 1 days);
assertEq(expectedSessionDuration, 4 weeks, "Session should be set for 4 weeks");
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
uint256 actualSessionDuration = block.timestamp - sessionStartTime;
console2.log("Time elapsed before graduation:", actualSessionDuration);
console2.log("Time remaining in session that was skipped:", expectedSessionEnd - block.timestamp);
console2.log("Percentage of session completed:", (actualSessionDuration * 100) / expectedSessionDuration, "%");
// Demonstrate the vulnerability - graduation happened before session end time
assertTrue(block.timestamp < expectedSessionEnd, "Graduation should not be possible before session ends");
assertTrue(actualSessionDuration < expectedSessionDuration, "Session ended prematurely");
assertTrue(actualSessionDuration < 1 days, "Session lasted less than a day instead of 4 weeks");
}

Recommendations

Modify the graduateAndUpgrade function to require that the session end date has been reached:

function graduateAndUpgrade(
address _levelTwo,
bytes memory
) public onlyPrincipal {
// q does it follow CEI
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
// Ensure the session has run for its full intended duration
require(block.timestamp >= sessionEnd, "Session duration not yet completed");
// ......Rest of the code
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 5 months 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

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