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

Unlimited Review Exploitation & Array Bounds Safety Issues

teheelaa

Summary

Two key vulnerabilities exist in the LevelOne contract related to review counting and array manipulation

Vulnerability Details

  1. Unlimited Review Exploitation

​
// Current vulnerable implementation in LevelOne.sol
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");
​
if (!review) {
studentScore[_student] -= 10;
}
​
lastReviewTime[_student] = block.timestamp;
// Missing: reviewCount[_student]++;
}

PoC Test:

function testUnlimitedReviews() public {
// Setup
vm.prank(principal);
levelOne.addTeacher(teacher);
levelOne.startSession();
// Exploit
for(uint i = 0; i < 10; i++) { // Should be limited to 5
vm.warp(block.timestamp + 1 weeks);
vm.prank(teacher);
levelOne.giveReview(student, false); // Can give more than 5 reviews
}
// Student score can be reduced to 0 or below
assert(levelOne.studentScore(student) <= 0);
}
​

  1. Array Bounds Safety

​
// Current implementation in LevelOne.sol
function removeTeacher(address _teacher) public onlyPrincipal {
uint256 teacherLength = listOfTeachers.length;
for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
break;
}
}
}

PoC Test:

function testArrayBoundsSafety() public {
// Setup empty teacher list
vm.prank(principal);
levelOne.removeTeacher(address(0x1)); // Could cause issues if array is empty
// Test with single teacher
vm.prank(principal);
levelOne.addTeacher(address(0x1));
levelOne.removeTeacher(address(0x1));
// Verify length
assert(levelOne.getTotalTeachers() == 0);
}
​

Impact

  1. Unlimited Reviews:

  • Teachers can give unlimited negative reviews

  • Student scores can be manipulated beyond intended limits

  • Breaks the 5-review limit business logic

  • Could unfairly impact student graduation eligibility

  1. Array Bounds:

  • Potential for out-of-bounds access

  • Risk of array manipulation errors

  • Could lead to contract state inconsistencies

Tools Used

  • Foundry/Forge for testing

  • Manual code review

Recommendations

  1. Fix Review Counter:

function giveReview(address _student, bool review) public onlyTeacher {
// ... existing checks ...
if (!review) {
studentScore[_student] -= 10;
}
reviewCount[_student]++; // Add counter increment
lastReviewTime[_student] = block.timestamp;
}
​

  1. Add Array Safety:

function removeTeacher(address _teacher) public onlyPrincipal {
if (listOfTeachers.length == 0) revert EmptyTeacherList();
uint256 teacherLength = listOfTeachers.length;
bool found = false;
for (uint256 n = 0; n < teacherLength; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[teacherLength - 1];
listOfTeachers.pop();
found = true;
break;
}
}
require(found, "Teacher not found in list");
}
​


Updates

Lead Judging Commences

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