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

reviewCount Not Incrementing After Reviews

codeaudit0x1

Summary

The giveReview A function in the smart contract contains a logical vulnerability that allows multiple reviews to be counted without incrementing the reviewCount mapping. As a result, the require(reviewCount[_student] < 5, ...) The condition is effectively bypassed, allowing a teacher to submit unlimited reviews for a student, especially negative ones, which can unfairly manipulate student scores.

Vulnerability Details

The function giveReview(address _student, bool review) is designed to allow a teacher to give one review per week, up to a total of 4 reviews per student. However, the code does not increment the reviewCount[_student] mapping after a review is given:

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

This check validates that a student hasn't received more than 5 reviews, but since reviewCount[_student] is never updated, the condition will always pass, allowing infinite reviews to be submitted by the teacher.

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

This means if false (bad review) is passed multiple times, the studentScore can be arbitrarily decreased. The absence of a reviewCount[_student]++ line or similar tracking mechanism is the core issue.

Proof Of Concept

The following test case demonstrates the issue clearly:

function test_NotReviewCountIncreased() public {
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// check for student add
assert(levelOneProxy.getTotalStudents() == 1);
​
// add teacher
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
vm.stopPrank();
​
// check for teacher add
assert(levelOneProxy.getTotalTeachers() == 1);
​
vm.warp(block.timestamp + 1 weeks);
vm.prank(alice);
levelOneProxy.giveReview(clara, false);
assert(levelOneProxy.studentScore(clara) == 90);
​
vm.warp(block.timestamp + 1 weeks);
vm.prank(alice);
levelOneProxy.giveReview(clara, false);
assert(levelOneProxy.studentScore(clara) == 80);
​
// Failing assertion: expected 2 reviews, got 0
assertEq(levelOneProxy.reviewCount(clara), 2);
}
​

Console Output

[320314] LevelOneAndGraduateTest::test_NotReviewCountIncreased()
├─ [0] VM::startPrank(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ └─ ← [Return]
├─ [24739] MockUSDC::approve(ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], spender: ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [157516] ERC1967Proxy::enroll()
│ ├─ [152629] LevelOne::enroll() [delegatecall]
│ │ ├─ [30869] MockUSDC::transferFrom(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], to: ERC1967Proxy: [0xA8452Ec99ce0C64f20701dB7dD3abDb607c00496], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [750] ERC1967Proxy::getTotalStudents() [staticcall]
│ ├─ [360] LevelOne::getTotalStudents() [delegatecall]
│ │ └─ ← [Return] 1
│ └─ ← [Return] 1
├─ [0] VM::startPrank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [71191] ERC1967Proxy::addTeacher(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ ├─ [70801] LevelOne::addTeacher(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3]) [delegatecall]
│ │ ├─ emit TeacherAdded(: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [772] ERC1967Proxy::getTotalTeachers() [staticcall]
│ ├─ [382] LevelOne::getTotalTeachers() [delegatecall]
│ │ └─ ← [Return] 1
│ └─ ← [Return] 1
├─ [0] VM::warp(604801 [6.048e5])
│ └─ ← [Return]
├─ [0] VM::prank(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ └─ ← [Return]
├─ [28336] ERC1967Proxy::giveReview(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], false)
│ ├─ [27943] LevelOne::giveReview(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], false) [delegatecall]
│ │ ├─ emit ReviewGiven(student: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], review: false, studentScore: 90)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [958] ERC1967Proxy::studentScore(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [staticcall]
│ ├─ [565] LevelOne::studentScore(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [delegatecall]
│ │ └─ ← [Return] 90
│ └─ ← [Return] 90
├─ [0] VM::warp(1209601 [1.209e6])
│ └─ ← [Return]
├─ [0] VM::prank(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ └─ ← [Return]
├─ [4436] ERC1967Proxy::giveReview(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], false)
│ ├─ [4043] LevelOne::giveReview(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], false) [delegatecall]
│ │ ├─ emit ReviewGiven(student: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], review: false, studentScore: 80)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [958] ERC1967Proxy::studentScore(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [staticcall]
│ ├─ [565] LevelOne::studentScore(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [delegatecall]
│ │ └─ ← [Return] 80
│ └─ ← [Return] 80
├─ [1024] ERC1967Proxy::reviewCount(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [staticcall]
│ ├─ [631] LevelOne::reviewCount(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879]) [delegatecall]
│ │ └─ ← [Return] 0
│ └─ ← [Return] 0
├─ [0] VM::assertEq(0, 2) [staticcall]
│ └─ ← [Revert] assertion failed: 0 != 2
└─ ← [Revert] assertion failed: 0 != 2
​
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 5.29ms (445.11µs CPU time)
​
Ran 1 test suite in 1.32s (5.29ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest
[FAIL. Reason: assertion failed: 0 != 2] test_NotReviewCountIncreased() (gas: 320314)

Impact

The failure to increment reviewCount[_student] in the giveReview function has critical implications on the overall integrity and progression of the school system:

  • Bypasses Weekly Review Enforcement

  • Violates Graduation Preconditions

  • Breaks Session-End Validation Logic

Tools Used

Manual Review

Recommendations

Fix the Core Logic: Add the following line inside the giveReview function:

reviewCount[_student]++;
Updates

Lead Judging Commences

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