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

[H-8] Missing Graduation Requirements Validation in LevelOne

nymuelis

Severity

High

Impact

The graduateAndUpgrade() function in LevelOne does not enforce critical graduation requirements specified in the project readme. This allows premature system upgrades, bypassing review requirements, and potentially graduating students who should have failed. This breaks three explicit invariants listed in the readme.

Additionally, the review counting system is broken because the reviewCount is never incremented in the giveReview function, making it impossible to properly track and enforce review requirements.

Description

According to the readme, the following invariants should be enforced:

  1. "Students must have gotten all reviews before system upgrade"

  2. "Any student who doesn't meet the cutOffScore should not be upgraded"

  3. "System upgrade cannot take place unless school's sessionEnd has reached"

The graduateAndUpgrade() function in LevelOne does not validate any of these requirements:

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);
​
// Payment distribution code...
}

Issues:

  1. No check that block.timestamp >= sessionEnd to ensure the session is complete

  2. No validation that each student has received 4 reviews (one per week for 4 weeks)

  3. No filtering of students based on the cutOffScore requirement

Furthermore, the giveReview function has a critical bug - it checks the review count but never increments it:

function giveReview(address _student, bool review) public onlyTeacher {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
require(reviewCount[_student] < 5, "Student review count exceeded!!!");
require(block.timestamp >= lastReviewTime[_student] + reviewTime, "Reviews can only be given once per week");
​
// where `false` is a bad review and true is a good review
if (!review) {
studentScore[_student] -= 10;
}
​
// Update last review time
lastReviewTime[_student] = block.timestamp;
// Missing: reviewCount[_student]++
​
emit ReviewGiven(_student, review, studentScore[_student]);
}

This means that even if validation for review counts were added to graduateAndUpgrade(), it would be ineffective since the counts are never updated.

The principal can call this function at any time (even immediately after starting a session), bypassing the entire school process and potentially graduating students who should have failed.

Proof of Concept

function test_PrematureGraduation() public {
// Setup school with principal, teacher, and student
address principal = address(1);
address teacher = address(2);
address student = address(3);
// Initialize school
school.initialize(principal, 100, address(usdc));
// Add teacher and start session
vm.prank(principal);
school.addTeacher(teacher);
// Student enrolls
vm.startPrank(student);
usdc.approve(address(school), 100);
school.enroll();
vm.stopPrank();
// Start session
vm.prank(principal);
school.startSession(70); // Set cutoff score to 70
// No reviews given, session not ended
// Principal can still call graduateAndUpgrade immediately
vm.prank(principal);
school.graduateAndUpgrade(address(levelTwo), "");
// School is upgraded despite no reviews and session not ending
assertTrue(levelTwo.isStudent(student));
}

Tools Used

Manual code review, custom test scenario

Recommended Mitigation

Add validation checks to enforce all required invariants and fix the review count increment:

function giveReview(address _student, bool review) public onlyTeacher {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
require(reviewCount[_student] < 5, "Student review count exceeded!!!");
require(block.timestamp >= lastReviewTime[_student] + reviewTime, "Reviews can only be given once per week");
​
// where `false` is a bad review and true is a good review
if (!review) {
studentScore[_student] -= 10;
}
​
// Update last review time
lastReviewTime[_student] = block.timestamp;
+ // Increment the review count
+ reviewCount[_student]++;
​
emit ReviewGiven(_student, review, studentScore[_student]);
}
​
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ // Check that session has ended
+ require(block.timestamp >= sessionEnd, "Session not ended yet");
+ // Check all students have received all reviews
+ for (uint256 i = 0; i < listOfStudents.length; i++) {
+ require(
+ reviewCount[listOfStudents[i]] >= 4,
+ "Not all students have received required reviews"
+ );
+ }
uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
_authorizeUpgrade(_levelTwo);
// Payment code...
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 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.

Support

FAQs

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