Summary
The reviewCount variable is used to limit the number of reviews a student can receive, but it is never incremented when a review is given. This completely breaks the review limitation mechanism, allowing unlimited reviews for each student.
Vulnerability Details
function giveReview(address _student, bool review) public onlyTeacher {
require(reviewCount[_student] < 5, "Student review count exceeded!!!");
require(
block.timestamp >= lastReviewTime[_student] + reviewTime,
"Reviews can only be given once per week"
);
if (!review) {
studentScore[_student] -= 10;
}
lastReviewTime[_student] = block.timestamp;
emit ReviewGiven(_student, review, studentScore[_student]);
}
The issue is that while the function checks if reviewCount[_student] < 5, it never increments reviewCount[_student] after giving a review. Since the counter is never incremented, the check will continue to pass indefinitely, making the 5-review limit ineffective.
Impact
Teachers can give unlimited negative reviews to a student, potentially reducing their score to 0 or a very low value. This undermines the intended limitation of 5 reviews per student and could allow malicious teachers to unfairly penalize students.
Tools Used
Recommendations
function giveReview(address _student, bool review) public onlyTeacher {
if (!review) {
studentScore[_student] -= 10;
}
lastReviewTime[_student] = block.timestamp;
reviewCount[_student]++;
emit ReviewGiven(_student, review, studentScore[_student]);
}
This change will ensure that the review count limit works as intended, preventing teachers from giving more than 5 reviews to any student.
POC
function test_ReviewCountNotIncremented() public {
_teachersAdded();
_studentsEnrolled();
vm.prank(principal);
levelOneProxy.startSession(70);
address student = levelOneProxy.getListOfStudents()[0];
uint256 initialScore = levelOneProxy.studentScore(student);
console.log("Initial student score:", initialScore);
vm.startPrank(alice);
for (uint256 i = 0; i < 6; i++) {
vm.warp(block.timestamp + 1 weeks + 1 days);
levelOneProxy.giveReview(student, false);
console.log(
"Review #",
i + 1,
"given. Student score:",
levelOneProxy.studentScore(student)
);
}
vm.stopPrank();
uint256 expectedScoreIfLimited = initialScore - 50;
uint256 actualFinalScore = levelOneProxy.studentScore(student);
console.log(
"Expected score if limited to 5 reviews:",
expectedScoreIfLimited
);
console.log("Actual final score after 6 reviews:", actualFinalScore);
assertTrue(
actualFinalScore < expectedScoreIfLimited,
"Expected student to receive more than 5 reviews, but limit seems to be working"
);
}
Output
-> % forge test --mt test_ReviewCountNotIncremented -vvv
[⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 598.94ms
Compiler run successful!
Ran 1 test for test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest
[PASS] test_ReviewCountNotIncremented() (gas: 1013520)
Logs:
Initial student score: 100
Review # 1 given. Student score: 90
Review # 2 given. Student score: 80
Review # 3 given. Student score: 70
Review # 4 given. Student score: 60
Review # 5 given. Student score: 50
Review # 6 given. Student score: 40
Expected score if limited to 5 reviews: 50
Actual final score after 6 reviews: 40
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 17.16ms (3.76ms CPU time)
Ran 1 test suite in 103.79ms (17.16ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
