Hawk High

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

Review Count Never Incremented Allowing Unlimited Student Reviews

Summary

The giveReview function in the LevelOne contract contains a critical oversight: while it checks if a student has received fewer than 5 reviews, it never increments the reviewCount mapping after performing a review. This allows teachers to perform an unlimited number of reviews for the same student (subject only to the weekly time limitation), bypassing the intended limit of 4 reviews per student.

Vulnerability Details

In the giveReview function, the contract checks reviewCount[_student] < 5 to ensure a student hasn't received too many reviews:

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

The issue is that while the function checks if the review count exceeds the limit, it never increments reviewCount[_student] after performing the review. As a result, the check will always pass as long as the weekly time limitation between reviews is respected.

Impact

  • A student can receive an unlimited number of reviews instead of the intended maximum of 5

  • Teachers can repeatedly give negative reviews to the same student, potentially reducing their score to an extremely low value

  • This could be used maliciously by teachers to prevent certain students from graduating by repeatedly giving negative reviews

  • The business logic of having a maximum review limit is entirely bypassed

Tools Used

  • Manual code review

  • Custom test case to demonstrate the vulnerability

Proof of Concept

Add the following test to your test file to demonstrate the issue:

function test_confirm_can_give_more_than_4_review() public schoolInSession {
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(alice);
for(uint8 i=0; i < 6; i++){
levelOneProxy.giveReview(harriet, false);
vm.warp(block.timestamp + 1 weeks);
}
assert(levelOneProxy.studentScore(harriet) == 40);
}

Recommendations

Modify the giveReview function to increment the review count after each review:

function giveReview(address _student, bool review) public onlyTeacher {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
require(reviewCount[_student] < 4, "Student review count exceeded!!!");
require(
block.timestamp >= lastReviewTime[_student] + reviewTime,
"Reviews can only be given once per week"
);
// Increment the review count
reviewCount[_student]++;
// 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;
emit ReviewGiven(_student, review, studentScore[_student]);
}

Additionally, consider adding a function to view a student's current review count for transparency.

Updates

Lead Judging Commences

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