Hawk High

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

Unbounded Review Capability Due to Non-Incrementing Counter


Severity: High

Likelihood: High

Summary

The LevelOne contract implements a review system that is meant to limit teachers to giving at most 5 reviews per student. However, due to a critical implementation error, the review counter is never incremented after a review is given, allowing teachers to provide unlimited reviews to students.

Vulnerability Details

The contract defines a private mapping to track review counts:

mapping(address => uint256) private reviewCount;

In the giveReview function, there is a validation check:

require(reviewCount[_student] < 5, "Student review count exceeded!!!");

The vulnerability exists because after this check passes, the function never updates the reviewCount mapping for the student. Since reviewCount[_student] defaults to 0 for new mappings in Solidity and is never modified, this check will always pass.

Impact

This vulnerability has several serious implications:

  1. Score Manipulation: Teachers can continuously give negative reviews to students, potentially reducing their score to 0

  2. System Invariant Violation: The system is designed with an expectation that students receive a maximum of 5 reviews, but this constraint is broken and there is no sessionEnd check to stop more than 4 reviews , so the student gets infinite times.

  3. Graduation Criteria Interference: If student graduation or advancement depends on maintaining certain scores, this vulnerability allows arbitrary interference with those scores

  4. Fairness Disruption: The education system model assumes fair and limited evaluation, but this vulnerability enables selective targeting of specific students

Code Reference

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

Proof of Concept

Consider this scenario:

  1. A teacher calls giveReview(studentAddress, false) for the first time

  2. The reviewCount[studentAddress] is 0 (default value), so the check passes

  3. The student's score is reduced by 10

  4. After one week, the teacher can again call giveReview(studentAddress, false)

  5. Since reviewCount[studentAddress] is still 0, the check passes again

  6. This process can be repeated indefinitely, once per week

Recommendations

Add a statement to increment the review counter after each successful review:

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;
}
// Fix: Increment the review counter
reviewCount[_student]++;
// 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
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.