Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: low
Invalid

Incorrect Review Limit in `LevelOne::giveReview` Breaks Protocol Invariant Defined by The Protocol.

rubencrxz

Summary

The LevelOne::giveReview function contains an issue regarding the number of reviews a student can receive. While the protocol defines an invariant that each student should receive only four reviews, the current implementation allows five.

Vulnerability Details

The protocol establishes that each student must receive a maximum of four reviews. However, the LevelOne::giveReview function checks whether the student's reviewCount is less than five (< 5) before allowing another review, effectively enabling a fifth review. This breaks the intended invariant of the system.

Impact

This issue violates a core protocol invariant and may lead to unintended behavior. Although reviews are restricted to one per week as enforced by the immutable LevelOne::reviewTime and sessions are expected to last four weeks as defined by LevelOne::sessionEnd making this issue not very likely to happen, this discrepancy could become problematic if these values change or are misinterpreted. Additionally, it harms code clarity and may confuse future maintainers or integrators.

The issue can be confirmed by adding the following proof of code to the LevelOneAngGraduateTest.t.sol contract. The test will pass, showing that five reviews are allowed.

Proof of Concept

  1. Teacher alice submits five reviews for student fin.

  2. The test passes, as the code allows for five reviews.

Proof of Code

function test_review_count_limit() public schoolInSession {
vm.prank(alice);
levelOneProxy.giveReview(fin, false);
​
assert(levelOneProxy.reviewCount(fin) == 1);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.prank(alice);
levelOneProxy.giveReview(fin, false);
​
assert(levelOneProxy.reviewCount(fin) == 2);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.prank(alice);
levelOneProxy.giveReview(fin, false);
​
assert(levelOneProxy.reviewCount(fin) == 3);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.prank(alice);
levelOneProxy.giveReview(fin, false);
​
assert(levelOneProxy.reviewCount(fin) == 4);
​
vm.warp(block.timestamp + 1 weeks);
​
vm.prank(alice);
levelOneProxy.giveReview(fin, false);
​
assert(levelOneProxy.reviewCount(fin) == 5);

Tools Used

This issue was identified through manual review.

Recommendations

Modify the conditional in the LevelOne::giveReview function to enforce the intended limit of four reviews per student, modifying the require statement:

function giveReview(address _student, bool review) public onlyTeacher {
if (!isStudent[_student]) {
revert HH__StudentDoesNotExist();
}
- require(reviewCount[_student] < 5, "Student review count exceeded!!!");
+ require(reviewCount[_student] < 4, "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]);
}

Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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