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

Insufficient Score Validation in `graduateAndUpgrade` Function

elyas6126

Summary

The LevelOne::graduateAndUpgrade function fails to properly validate student scores during the graduation process, allowing students with scores below the minimum cutoff to graduate and progress to the next level.

Vulnerability Details

The contract's graduation mechanism is intended to filter out students who don't meet the minimum score requirement (set during session start). However, the implementation doesn't correctly check each student's score against this threshold. As demonstrated in the test, a student with multiple negative reviews and a score below the cutoff threshold remains in the student list after graduation, instead of being removed.

PoC

function test_graduate_function_donot_consider_students_score() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(eli);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(fin);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(grey);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(harriet);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.prank(principal);
levelOneProxy.startSession(70);
vm.stopPrank();
​
vm.warp(block.timestamp + 1 weeks);
​
vm.startPrank(alice);
levelOneProxy.giveReview(fin, false);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.startPrank(alice);
levelOneProxy.giveReview(fin, false);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.startPrank(alice);
levelOneProxy.giveReview(fin, false);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.startPrank(alice);
levelOneProxy.giveReview(fin, false);
​
// I added getStudentScore function to the LevelOne contract to retrieve Fin's score.
uint256 finScore = levelOneProxy.getStudentScore(fin);
console2.log("fin's score:", finScore);
assertLt(finScore, 70, "Test requires fin's score to be below cutoff");
​
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.startPrank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// Fin should not be included in the student list because their score is below the cutoff.
address[] memory students = levelOneProxy.getListOfStudents();
bool finFound = false;
for (uint256 i = 0; i < students.length; i++) {
if (students[i] == fin) {
finFound = true;
break;
}
}
assertTrue(finFound,"Bug confirmed: Student with score below cutoff still in list after graduation" );
}

Impact

This vulnerability undermines the entire academic merit system implemented in the contract. Students who haven't met the minimum requirements can unfairly advance to the next level, defeating the purpose of having performance evaluations and score thresholds. This could lead to:

  1. Unqualified students progressing through the system

  2. Devaluation of the educational certification

  3. Loss of trust in the fairness of the academic system

  4. Potential financial implications if graduation unlocks monetary rewards or reduces fees

Tools Used

Manual Review, Foundry

Recommendations

The fix ensures graduateAndUpgrade in LevelOne.sol only graduates students with scores at or above cutOffScore. It loops through listOfStudents, removing students with scores below the threshold by swapping with the last element and popping it, preserving array integrity. The else increments i to avoid index issues, ensuring only qualified students remain.

function graduateAndUpgrade(address _LevelTwo, bytes memory) public onlyPrincipal {
.
.
.
+ uint256 studentLength = listOfStudents.length
+ for(uint256 i, i < studentLength, i++) {
+ if(studentScore[listOfStudents[i]] < cutOffScore) {
+ listOfStudents[i] = listOfStudents[studentLength - 1];
+ listOfstudents.pop();
+ } else {
+ i++
+ }
+ }
_authorizeUpgrade(_levelTwo);
.
.
}
Updates

Lead Judging Commences

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