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

Students can graduate regardless of their score.

5am

Summary

There is no check in graduateAndUpgrade() that a student's score is above the cutOffScore.

Vulnerability Details

This PoC only works if the vulnerablities affecting the storage layout and invalid implementation are fixed, otherwise we can't upgrade the contract and therefore cannot check it's state.

PoC:

function test_cutOffScore_is_not_enforced() public {
// We have to mint some extra USDC to account for a miscalculation in teachers' wages
usdc.mint(address(levelOneProxy), schoolFees);
​
// Add 4 teachers to the school to give reviews
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
levelOneProxy.addTeacher(charlie);
levelOneProxy.addTeacher(dave);
vm.stopPrank();
​
// Enroll a student
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// Start school session with a cutOffScore of 70
vm.startPrank(principal);
levelOneProxy.startSession(70);
vm.stopPrank();
​
// Warp forward 1 week
vm.warp(block.timestamp + 1 weeks);
​
// 4 teachers leave 4 negative reviews
vm.startPrank(alice);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(bob);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(charlie);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(dave);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
​
// Check student score is below cutOffScore
assert(levelOneProxy.studentScore(clara) < 70);
console2.log("Student score: ", levelOneProxy.studentScore(clara));
console2.log("Session cutOffScore: ", levelOneProxy.cutOffScore());
​
// Principal calls graduateAndUpgrade
vm.startPrank(principal);
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// Principal performs actual upgrade
// This passes after fixing wrong storage layout and invalid implementation.
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
​
// Check that the student is in the new contract
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
assert(levelTwoProxy.getTotalStudents() == 1);
console2.log("Total students in new contract: ", levelTwoProxy.getTotalStudents());
}

Impact

By not checking a student's score is above the cutOffScore, we allow any student to graduate regardless of how many bad reviews they get. This invalidates the protocol's core functionality.

Tools Used

Manual review

Recommendations

Expel students that don't meet the cutOffScore.

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
+ for (uint256 n = 0; n < listOfStudents.length; n++) {
+ if (studentScore[listOfStudents[n]] < cutOffScore) {
+ expel(listOfStudents[n]);
+ }
+ }
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 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.