Hawk High

First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

expel() does not clean up student data

Summary

The expel() function in the LevelOne contract removes a student from the students mapping but fails to clean up associated data such as the student's presence in the studentsWhoPaid array. This can lead to stale data persisting in the contract state, potentially affecting logic that relies on an accurate list of enrolled or paying students.


Vulnerability Details

In the LevelOne contract, the expel() function is implemented to remove a student from the course:

function expel(address student) external onlyTeacher {
delete students[student];
emit StudentExpelled(student);
}

While this clears the students[student] mapping entry, it does not remove the student’s address from the studentsWhoPaid array. This leads to several problems:

  • studentsWhoPaid will still contain the expelled student's address, which may be incorrectly used in future operations like graduation or upgrade.

  • If graduation logic pays out bursaries or other rewards based on the studentsWhoPaid array, expelled students may still receive benefits despite being expelled.

  • The studentsWhoPaid array size becomes inaccurate, affecting statistics or logic that relies on student count.

  • The presence of stale data increases gas costs for iteration and risks introducing bugs in any logic that assumes all students in the array are valid.


Impact

  • Unintended Payouts: Expelled students may still receive bursary payments or rewards.

  • State Pollution: The contract state becomes cluttered with invalid entries, increasing gas costs and reducing clarity.

  • Logical Inconsistency: Systems or functions relying on the studentsWhoPaid array may behave incorrectly.

  • Potential for Exploitation: A malicious actor could pay, then get expelled and still benefit from residual presence in studentsWhoPaid.


Tools Used

  • Manual review

  • Solidity knowledge

  • Static reasoning about array and mapping interactions


Recommendations

  1. Remove Expelled Students From studentsWhoPaid:
    Use a helper function to remove the address from the array:

    function _removeFromStudentsWhoPaid(address student) internal {
    for (uint256 i = 0; i < studentsWhoPaid.length; i++) {
    if (studentsWhoPaid[i] == student) {
    studentsWhoPaid[i] = studentsWhoPaid[studentsWhoPaid.length - 1];
    studentsWhoPaid.pop();
    break;
    }
    }
    }
  2. Update expel() to Clean Up All Student Data:

    function expel(address student) external onlyTeacher {
    delete students[student];
    _removeFromStudentsWhoPaid(student);
    emit StudentExpelled(student);
    }
  3. Add Tests:
    Ensure the test suite covers expelling students who already paid and verifies they are removed from relevant data structures.

  4. Document Data Lifecycle:
    Include clear documentation on how and where student-related data is stored, modified, and cleaned up.

Updates

Lead Judging Commences

yeahchibyke Lead Judge
6 months ago
yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.