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

Missing functions in LevelTwo break critical invariants

mishoko

Summary

LevelTwo contract is missing critical functions, modifiers, events, error declaration that exist in LevelOne, including addTeacher, removeTeacher, expel, startSession, and giveReview. This creates a severe functionality gap and potential security issues.

Vulnerability Details

The missing functions in LevelTwo include:

  1. Access Control Functions:

    • addTeacher

    • removeTeacher

    • expel

    • startSession

  2. Core Business Logic:

    • giveReview (critical for student scoring)

    • getSessionStatus

    • getSessionEnd

    • getSchoolFeesCost

  3. Financial Functions:

    • graduateAndUpgrade (handles teacher and principal payments)

  4. Modifiers (all)

  5. Events (all)

  6. Errors (all)

This creates several issues:

  1. Teachers cannot be added or removed after upgrade

  2. Students cannot be expelled

  3. New sessions cannot be started

  4. Reviews cannot be given

  5. Teachers and principal cannot be paid

  6. Critical state checks are missing

Key invariants that are broken:

  1. Access Control Invariants:

  • onlyPrincipal checks for critical functions

  • onlyTeacher checks for review functions

  • notYetInSession checks for enrollment

  1. State Invariants:

  • Student score bounds (100 to 0)

  • Review frequency limits (once per week)

  • Maximum review count (5 reviews)

  • Session timing constraints

  1. Financial Invariants:

  • Teacher wage distribution (35%)

  • Principal wage distribution (5%)

  • Bursary management

Impact

The impact is severe:

  1. Functionality Loss: Core school operations become impossible after upgrade

  2. Financial Impact:

    • Teachers cannot be paid

    • Principal cannot be paid

    • Bursary funds become locked

  3. Access Control Issues:

    • No way to manage teachers

    • No way to manage students

    • No way to control sessions

  4. Business Logic Disruption:

    • Student scoring system breaks

    • Session management breaks

    • School operations halt

Tools Used

  • Manual code review

  • Function comparison analysis

Recommendations

  1. Implement all missing functions in LevelTwo:

contract LevelTwo is Initializable, UUPSUpgradeable {
// ... existing code ...
​
function addTeacher(address _teacher) public onlyPrincipal notYetInSession {
// Implementation
}
​
function removeTeacher(address _teacher) public onlyPrincipal {
// Implementation
}
​
function expel(address _student) public onlyPrincipal {
// Implementation
}
​
function startSession(uint256 _cutOffScore) public onlyPrincipal notYetInSession {
// Implementation
}
​
function giveReview(address _student, bool review) public onlyTeacher {
// Implementation
}
​
function getSessionStatus() external view returns (bool) {
return inSession;
}
​
function getSessionEnd() external view returns (uint256) {
return sessionEnd;
}
​
function getSchoolFeesCost() external view returns (uint256) {
return schoolFees;
}
​
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
// Implementation
}
}

  1. Add proper access control modifiers:

modifier onlyPrincipal() {
require(msg.sender == principal, "Not principal");
_;
}
​
modifier onlyTeacher() {
require(isTeacher[msg.sender], "Not teacher");
_;
}
​
modifier notYetInSession() {
require(!inSession, "Already in session");
_;
}

  1. Add missing state variables:

uint256 schoolFees;
uint256 public immutable reviewTime = 1 weeks;
mapping(address => uint256) private reviewCount;
mapping(address => uint256) private lastReviewTime;

  1. Add proper error handling:

error HH__NotPrincipal();
error HH__NotTeacher();
error HH__ZeroAddress();
error HH__TeacherExists();
error HH__StudentExists();
error HH__TeacherDoesNotExist();
error HH__StudentDoesNotExist();
error HH__AlreadyInSession();
error HH__ZeroValue();
error HH__HawkHighFeesNotPaid();
error HH__NotAllowed();

  1. Add proper events:

event TeacherAdded(address indexed);
event TeacherRemoved(address indexed);
event Enrolled(address indexed);
event Expelled(address indexed);
event SchoolInSession(uint256 indexed startTime, uint256 indexed endTime);
event ReviewGiven(address indexed student, bool indexed review, uint256 indexed studentScore);
event Graduated(address indexed levelTwo);

This vulnerability is particularly dangerous because it combines with the previous vulnerabilities (storage collision and missing UUPS inheritance) to create a perfect storm of issues that could completely break the contract's functionality after upgrade.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!