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

The `LevelOne::graduateAndUpgrade` ignores students review count, students score and is it past sessionEnd, breaking several invariants

toddorov

[H-2] The LevelOne::graduateAndUpgrade ignores students review count, students score and is it past sessionEnd, breaking several invariants

Description: As described in the documentation, the system upgdade should occur only if any student has got 4 reviews, has met the cutOffScore and the sessionEnd has passed.

Impact: Breaking several invariants.

Proof of Concepts:

Code 1. Place the following in `LevelOne.sol`
function getStudenReviewCount(address _student) external view returns (uint256) {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
return reviewCount[_student];
}
​
function getStudentCutOffScore(address _student) external view returns (uint256) {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
return studentScore[_student];
}

  1. Place the following test in LevelOneAndGraduateTest.t.sol

function test_confirm_can_graduate_without_invariants() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
address[] memory students = levelOneProxy.getListOfStudents();
for (uint256 i = 0; i < students.length; i++) {
for (uint256 j = 0; j < 5; j++) {
vm.prank(alice);
vm.warp(block.timestamp + 1 weeks);
levelOneProxy.giveReview(students[i], false);
}
​
console2.log("Student: ", students[i]);
console2.log("Score: ", levelOneProxy.getStudentCutOffScore(address(students[i])));
console2.log("Review Count: ", levelOneProxy.getStudenReviewCount(address(students[i])));
}
​
console2.log("Session end: ", levelOneProxy.sessionEnd());
console2.log("Current time: ", block.timestamp);
​
assert(levelOneProxy.sessionEnd() < block.timestamp);
​
vm.startPrank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
}

You should receive the following or similar output:

Student: 0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879
Score: 50
Review Count: 0
Student: 0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE
Score: 50
Review Count: 0
Student: 0xF238496034cA4D476743d590ff3A66def743F9be
Score: 50
Review Count: 0
Student: 0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680
Score: 50
Review Count: 0
Student: 0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd
Score: 50
Review Count: 0
Student: 0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b
Score: 50
Review Count: 0
Session end: 2419201
Current time: 18144001

Recommended mitigation: With the given invariants in the documentation, the graduateAndUpgrade function should be modified to check the invariants before upgrading.

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
+ require(block.timestamp > sessionEnd, "Session not ended yet!!!");
+ uint256 studentsLength = listOfStudents.length;
​
+ for(uint256 n = 0; n < studentsLength; n++) {
+ if (studentScore[listOfStudents[n]] < cutOffScore) {
+ revert HH__NotAllowed();
+ }
+ }
​
+ for(uint256 n = 0; n < studentsLength; n++) {
+ if (reviewCount[listOfStudents[n]] < 5) {
+ revert HH__NotAllowed();
+ }
+ }
​
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
3 months ago
yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

reviewCount not updated

`reviewCount` for students is not updated after each review session

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:

reviewCount not updated

`reviewCount` for students is not updated after each review session

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.