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

Lack of checks in `LevelOne::graduateAndUpgrade` causes invariants of the school system is broken

37h3rn17y2

Summary

LevelOne::graduateAndUpgrade does not check for protocol invariants before graduating and upgrading school system. As such, critical system invariants are allowed to be broken during graduation and upgrades.

Vulnerability Details

From the project README, there are several invariants of the system

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)
Any student who doesn't meet the cutOffScore should not be upgraded
System upgrade cannot take place unless school's sessionEnd has reached

However, LevelOne::graduateAndUpgrade does not perform any checks prior to system upgrade. This results in the following impacts

  1. System upgrade can happen before students have gotten all 4 reviews

  2. Students not meeting the cutOffScore are still upgraded

  3. System upgrade can occur before sessionEnd has reached

PoC

Place the following into LevelOne|AndGraduateTest.t.sol and run

forge test --mt testBreakInvariants

function testBreakInvariants() public {
// Register teachers and students
_teachersAdded();
_studentsEnrolled();
​
// Session start
uint256 _cutOffScore = 100;
vm.prank(principal);
levelOneProxy.startSession(_cutOffScore);
​
// Teacher gives 1 bad review to each student
address[] memory listOfStudents = levelOneProxy.getListOfStudents();
vm.warp(block.timestamp + 1 weeks + 1);
vm.startPrank(alice);
for (uint8 i = 0; i < levelOneProxy.getTotalStudents(); i++){
levelOneProxy.giveReview(listOfStudents[i], false);
}
vm.stopPrank();
​
// Graduate and upgrade school system
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// Assert that invariants do not hold up
// 1. System upgrade can happen before students have gotten all 4 reviews
// Teacher has only given 1 review per student shown above
// 2. Students not meeting the cutOffScore are still upgraded
assertLt(levelOneProxy.studentScore(clara), levelOneProxy.cutOffScore());
assertLt(levelOneProxy.studentScore(dan), levelOneProxy.cutOffScore());
assertLt(levelOneProxy.studentScore(eli), levelOneProxy.cutOffScore());
assertLt(levelOneProxy.studentScore(fin), levelOneProxy.cutOffScore());
assertLt(levelOneProxy.studentScore(grey), levelOneProxy.cutOffScore());
assertLt(levelOneProxy.studentScore(harriet), levelOneProxy.cutOffScore());
// 3. System upgrade can occur before `sessionEnd` has reached
assertLt(block.timestamp, levelOneProxy.sessionEnd());
}

Impact

  1. System upgrade can happen before students have gotten all 4 reviews

  2. Students not meeting the cutOffScore are still upgraded

  3. System upgrade can occur before sessionEnd has reached

Impact: High, invariants of the school system are broken
Likelihood: High, principal will upgrade school system at the end of school session (after 4 weeks)
Severity: High

Tools Used

Manual review

Recommendations

Perform invariant checks before allowing school system upgrades

LevelOne::graduateAndUpgrade

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
+ // @note this is vulnerable to DoS attack, limit the max student enrolment in `LevelOne::enrol` to prevent attack
+ for (uint8 i = 0; i < listOfStudents.length; i++){
+ require(reviewCount[i] == 4, "Students have not gotten all 4 reviews");
+ // If student has less than cutoff score
+ // expel student
+ // remove from listOfStudents
+ // decrement i so the student replacing the expelled student in listOfStudents is not overlooked
+ if (studentScore[listOfStudents[i]] < cutOffScore){
+ isStudent[listOfStudents[i]] = false;
+ listOfStudents[i] = listOfStudents[listOfStudents.length - 1];
+ listOfStudents.pop();
+ i--;
+ }
+ }
+ require(block.timestamp > sessionEnd, "Session has not ended");
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);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
4 months ago
yeahchibyke Lead Judge 3 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

yeahchibyke Lead Judge 3 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

Appeal created

37h3rn17y2 Submitter
3 months ago
yeahchibyke Lead Judge
3 months ago
yeahchibyke Lead Judge 3 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.