Hawk High

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

Missing Invariant Checks in `graduateAndUpgrade` Violate `Session`,` Review`, and `Graduation` Rules

Summary

The LevelOne::graduateAndUpgrade function lacks critical checks to enforce protocol invariants, allowing the principal to upgrade the system prematurely (before sessionEnd), graduate students without 4 reviews, and upgrade students who don’t meet the cutOffScore. This violates multiple invariants: a 4-week session duration, one review per week, 4 reviews per student before upgrade, and exclusion of students below the cutoff score. These issues compromise the academic evaluation process and financial distribution.

Vulnerability Details

The graduateAndUpgrade function is responsible for upgrading the system to LevelTwo and distributing wages, but it fails to enforce the following invariants:

  1. Invariant 7: "System upgrade cannot take place unless school's sessionEnd has reached." The function does not check if block.timestamp >= sessionEnd.

  2. Invariant 4: "Students must have gotten all reviews before system upgrade. System upgrade should not occur if any student has not gotten 4 reviews (one for each week)." There is no check to ensure each student has reviewCount == 4.

  3. Invariant 6: "Any student who doesn't meet the cutOffScore should not be upgraded." The function does not filter students based on studentScore >= cutOffScore or update LevelTwo with eligible students.

  4. Invariant 5 (implied): "Students can only be reviewed once per week." While enforced in giveReview, the lack of review count validation in graduateAndUpgrade allows upgrades with incomplete or incorrect review cycles.

The function’s logic:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
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);
}

lacks checks for sessionEnd, reviewCount, or cutOffScore, allowing the principal to execute upgrades at any time, even immediately after starting a session, and without verifying student eligibility.

Proof of Concept

Add this function in LevelOneIAndGraduateTest.t.sol to check that no invarients are checked.

function test_graduateWithoutRequirements() public {
// Setup
vm.startPrank(principal);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
vm.startPrank(harriet);
usdc.approve(address(levelOneProxy), 5000e18);
levelOneProxy.enroll();
vm.stopPrank();
// Start session and immediately upgrade (violates all invariants)
vm.startPrank(principal);
levelOneProxy.startSession(70);
levelTwoImplementation = new LevelTwo();
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), "");
vm.stopPrank();
}
Output::
forge test --mt test_graduateWithoutRequirements -vvv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest
[PASS] test_graduateWithoutRequirements() (gas: 868423)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 21.67ms (906.90µs CPU time)
Ran 1 test suite in 207.90ms (21.67ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Incorrect Graduations: Students can graduate without completing the required 4 reviews or meeting the cutOffScore, violating Invariants 4 and 6. This undermines the academic evaluation process.

  • Premature Upgrades: Upgrading before sessionEnd (Invariant 7) disrupts the 4-week session cycle, potentially allowing incomplete review cycles and incorrect wage distributions.

  • Financial Disruption: Wages may be paid out before all reviews are completed, misaligning with the academic cycle and potentially overpaying teachers.

  • Protocol Integrity: The failure to enforce core invariants erodes trust in the system’s ability to manage academic progression and financial distributions fairly.

Tools Used

Foundry

Recommendations

+error HH__SessionNotEnded();
+error HH__IncompleteReviews();
+error HH__NotInSession();
function graduateAndUpgrade(address _levelTwo, bytes memory _data) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ if (block.timestamp < sessionEnd) {
+ revert HH__SessionNotEnded();
+ }
+ if (!inSession) {
+ revert HH__NotInSession();
+ }
// Check all students have 4 reviews
+ for (uint256 i = 0; i < listOfStudents.length; i++) {
+ if (reviewCount[listOfStudents[i]] != 4) {
+ revert HH__IncompleteReviews();
+ }
+ }
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
+ uint256 remainingBursary = bursary - (payPerTeacher * totalTeachers + principalPay);
_authorizeUpgrade(_levelTwo);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
// Update state for LevelTwo (only eligible students)
+ address[] memory eligibleStudents = new address[](listOfStudents.length);
+ uint256 eligibleCount = 0;
+ for (uint256 i = 0; i < listOfStudents.length; i++) {
+ address student = listOfStudents[i];
+ if (studentScore[student] >= cutOffScore) {
+ eligibleStudents[eligibleCount] = student;
+ eligibleCount++;
+ }
+ }
+ bytes memory initData = abi.encodeCall(
+ LevelTwo.graduate,
+ (principal, address(usdc), remainingBursary, eligibleStudents, eligibleCount)
+ );
+ inSession = false;
+ bursary = remainingBursary;
+ emit Graduated(_levelTwo);
}

In LevelTwo.sol

- function graduate() public reinitializer(2) {}
+ function graduateWithStudents(
+ address _principal,
+ address _usdc,
+ uint256 _bursary,
+ address[] memory _students,
+ uint256 _count
+) public reinitializer(2) {
+ principal = _principal;
+ usdc = IERC20(_usdc);
+ bursary = _bursary;
+ for (uint256 i = 0; i < _count; i++) {
+ listOfStudents.push(_students[i]);
+ isStudent[_students[i]] = true;
+ studentScore[_students[i]] = 100; // Reset score
+ }
+}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
7 months ago
yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

cut-off criteria not applied

All students are graduated when the graduation function is called as the cut-off criteria is not applied.

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.

Give us feedback!