Hawk High

First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Valid

Uninitialized variable `reviewCount` in `LevelOne.sol`

Summary

The variable reviewCount is never initialized, meaning no limit is enforced on the number of reviews a student can receive.

Vulnerability Details

The variable reviewCount is never updated within giveReview and therefore never initialized. It allows giveReview to be called an unlimited amount of times.

function test_review_limit_not_enforced() public {
// We have to mint some extra USDC to account for a miscalculation in teachers' wages
usdc.mint(address(levelOneProxy), schoolFees);
// Add 6 teachers to the school to give reviews
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
levelOneProxy.addTeacher(charlie);
levelOneProxy.addTeacher(dave);
levelOneProxy.addTeacher(eve);
levelOneProxy.addTeacher(frank);
vm.stopPrank();
// Enroll a student
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
// Start school session with a cutOffScore of 40
vm.startPrank(principal);
levelOneProxy.startSession(40);
vm.stopPrank();
// Warp forward 1 week
vm.warp(block.timestamp + 1 weeks);
// 6 teachers leave 6 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();
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(eve);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(frank);
levelOneProxy.giveReview(clara, false);
vm.stopPrank();
// Check that the student has 6 negative reviews (which would lead to a score of 40)
// We can't check reviewCount directly because it's never initialized
assert(levelOneProxy.studentScore(clara) == 40);
}

Impact

This means that there is no limit on the amount of reviews a single student can receive. If there is more than a week gap between a student's 4th review and the principle calling graduateAndUpgrade, a 5th teacher could give the student a negative review. This would bring the student's score down unfairly. If the principle waits another week the same thing could happen again with a 6th teacher.

Tools Used

Manual review

Recommendations

Increment reviewCount when calling giveReview.

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;
+ reviewCount[_student]++;
emit ReviewGiven(_student, review, studentScore[_student]);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

reviewCount not updated

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

Support

FAQs

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