Hawk High

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

[H-5] Absence of Access Controls in LevelTwo Contract

Severity

High

Impact

LevelTwo completely lacks the access control modifiers present in LevelOne, leaving all existing and future functionality unprotected. Without these controls, any address could call sensitive administrative functions, leading to unauthorized manipulation of the school system, potential fund theft, and complete compromise of the platform's integrity.

Description

In LevelOne, critical administrative functions are protected by modifier-based access controls:

// From LevelOne.sol
modifier onlyPrincipal() {
if (msg.sender != principal) {
revert HH__NotPrincipal();
}
_;
}
modifier onlyTeacher() {
if (!isTeacher[msg.sender]) {
revert HH__NotTeacher();
}
_;
}

However, LevelTwo lacks any of these access control mechanisms. While LevelTwo currently has minimal functionality, it inherits all the state variables from LevelOne, including the principal address and mappings for teachers and students. Without proper access control, once functionality is implemented:

  1. Anyone could call functions meant only for the principal

  2. Anyone could call functions meant only for teachers

  3. Anyone would be able to manage teachers, students, and school funds

  4. The entire role-based permission system would collapse

This is particularly concerning as 60% of the bursary funds are meant to be managed in LevelTwo, according to the protocol's documentation. Without access controls, these funds would be vulnerable to unauthorized manipulation.

Tools Used

Manual code review

Recommended Mitigation

Add the same access control modifiers to LevelTwo that exist in LevelOne:

// LevelTwo.sol
contract LevelTwo is Initializable, UUPSUpgradeable {
// ... existing code ...
+ ////////////////////////////////
+ ///// /////
+ ///// MODIFIERS /////
+ ///// /////
+ ////////////////////////////////
+ modifier onlyPrincipal() {
+ if (msg.sender != principal) {
+ revert HH__NotPrincipal();
+ }
+ _;
+ }
+
+ modifier onlyTeacher() {
+ if (!isTeacher[msg.sender]) {
+ revert HH__NotTeacher();
+ }
+ _;
+ }
+
+ modifier notYetInSession() {
+ if (inSession == true) {
+ revert HH__AlreadyInSession();
+ }
+ _;
+ }
// ... rest of contract ...
}

Additionally, ensure all sensitive functions include appropriate access modifiers when implementing them.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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