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

Uninitialized `reviewCount` in `giveReview` Permits Unlimited Reviews, Violating Review Limits

marutint10

Summary

The LevelOne::giveReview function checks reviewCount[_student] < 5 to limit student reviews but fails to increment reviewCount after a review is given. This renders the check ineffective, allowing teachers to submit unlimited reviews for a student, violating Invariant 4 ("Students must have gotten all reviews before system upgrade") and Invariant 5 (implied: "Students can only be reviewed once per week"). The issue also misaligns with the 4-review limit intended for a 4-week session, compromising the academic evaluation process.

Vulnerability Details

The giveReview function is designed to allow teachers to give weekly reviews (good or bad) to students, with a limit of one review per week and a maximum of 4 reviews per student over a 4-week session. The function includes:

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]);
}

  1. Unincremented reviewCount: The reviewCount[_student] mapping is never incremented, so it remains 0, making the reviewCount[_student] < 5 check always pass. This allows unlimited reviews.

  2. Incorrect Limit: The check uses < 5, allowing up to 5 reviews, which conflicts with Invariant 4’s requirement of exactly 4 reviews per student (one per week for 4 weeks).

  3. Invariant Violations:

    • Invariant 4: "Students must have gotten all reviews before system upgrade" implies exactly 4 reviews. Unlimited reviews skew studentScore and bypass this requirement.

    • Invariant 5 (implied): "Students can only be reviewed once per week" is enforced by the lastReviewTime check, but unlimited reviews within a week (if lastReviewTime is manipulated or bypassed) could occur without proper reviewCount tracking.

This bug allows teachers to excessively increase or decrease studentScore (e.g., reducing it by 10 per bad review indefinitely), undermining the fairness of the review process.

Proof of Concept

Add this test to LevelOneAndGraduateTest.t.sol to demonstrate that reviewCount is not incremented, allowing unlimited reviews.

function test_unlimitedReviewsDueToUninitializedReviewCount() public schoolInSession {
// Give 6 reviews to a student (should be limited to 4)
vm.startPrank(alice); // Alice is a teacher
for (uint i = 0; i < 6; i++) {
vm.warp(block.timestamp + 1 weeks); // Ensure weekly review timing
levelOneProxy.giveReview(harriet, false); // Bad review, score -= 10
}
vm.stopPrank();
​
// Check reviewCount and studentScore
assertEq(levelOneProxy.studentScore(harriet), 40, "Score should be 100 - 6*10 = 40");
}

Impact

Unlimited Reviews: Teachers can give unlimited reviews, manipulating studentScore excessively (e.g., reducing it to 0 with repeated bad reviews), which undermines Invariant 4’s requirement of exactly 4 reviews.

  • Academic Fairness Compromised: Students may fail to graduate due to unfair score reductions or pass with inflated scores, affecting Invariant 6 ("Students below cutOffScore should not be upgraded").

  • Protocol Integrity: The review process’s reliability is eroded, as stakeholders expect a fair, 4-review evaluation.

Tools Used

Foundry

Recommendations

Update giveReview to:

  1. Increment reviewCount[_student] after a review.

  2. Enforce a 4-review limit (not 5) to align with Invariant 4.

  3. Use custom errors for gas efficiency and clarity.

+error HH__MaxReviewsReached();
+error HH__ReviewTooSoon();
​
function giveReview(address _student, bool review) public onlyTeacher {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
​
- require(reviewCount[_student] < 5, "Student review count exceeded!!!");
+ if (reviewCount[_student] >= 4) {
+ revert HH__MaxReviewsReached();
+ }
- require(block.timestamp >= lastReviewTime[_student] + reviewTime, "Reviews can only be given once per week");
+ if (block.timestamp < lastReviewTime[_student] + reviewTime) {
+ revert HH__ReviewTooSoon();
+ }
​
if (!review) {
studentScore[_student] -= 10;
}
​
+ reviewCount[_student]++;
lastReviewTime[_student] = block.timestamp;
​
emit ReviewGiven(_student, review, studentScore[_student]);
}
Updates

Lead Judging Commences

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