Hawk High

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

Lack of Access Control in `LevelOne::initialize` Function, Allowing Anyone to Initialize and Upgrade `LevelOne` Contract

Summary

The LevelOne::initialize function lacks proper access control, allowing any user to call it and potentially take ownership of the contract, which could lead to its compromise or destruction.

Vulnerability Details

The LevelOne::initialize function is publicly accessible and does not restrict who can invoke it. As a result, any external actor can initialize the contract at any time.

Impact

This vulnerability allows a malicious actor to initialize the contract before its intended owner, potentially assigning themselves as the principal. Once in control, they can point the proxy to a malicious implementation, compromising the system’s integrity and functionality.

Proof of concept

The vulnerability can be exploited through the following steps:

  1. Deploy the LevelOne contract using a script (e.g., DeployLevelOne.s.sol) that does not perform initialization.

  2. An unauthorized actor, such as a disgruntled teacher alice, notices the contract is uninitialized.

  3. alice calls the initialize function and assigns herself as the principal.

  4. With the principal role, alice executes upgradeToAndCall, directing the proxy to a malicious EvilImplementation contract that includes a dummy doNothing function or other undesired logic.

The proxy is now pointing to malicious code, and control of the contract has effectively been seized.

Proof of code

  1. In the DeployLevelOne.s.sol deployment script, the LevelOne contract is deployed without calling the initialize function, leaving it vulnerable to unauthorized initialization:

function deployLevelOne() public returns (address) {
usdc = new MockUSDC();
vm.startBroadcast();
levelOneImplementation = new LevelOne();
proxy = new ERC1967Proxy(address(levelOneImplementation), "");
- LevelOne(address(proxy)).initialize(principal, schoolFees, address(usdc));
vm.stopBroadcast();
return address(proxy);
}

2.At the end of the LevelOneAndGraduate.t.sol file, a malicious implementation named EvilImplementation is defined. This contract contains a doNothing function to simulate arbitrary, undesired logic and mimics a valid UUPS proxy target by implementing proxiableUUID.

contract EvilImplementation {
function proxiableUUID() public pure returns (bytes32) {
return 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
}
function doNothing() external {
}
}

3.Then, inside the LevelOneAndGraduateTest contract, a test case demonstrates how a supposedly unauthorized user alice can initialize the proxy and redirect it to the malicious implementation:

function test_anyone_can_initialize_and_brick() public {
vm.startPrank(alice);
levelOneProxy.initialize(alice, schoolFees, address(usdc));
assert(levelOneProxy.getPrincipal() == alice);
EvilImplementation evil = new EvilImplementation();
levelOneProxy.upgradeToAndCall(
address(evil),
abi.encodeWithSignature("doNothing()")
);
vm.stopPrank();
vm.expectRevert();
levelOneProxy.getPrincipal();
}

Once this test passes, it proves that the contract was initialized by an unauthorized actor alice and then redirected to the malicious EvilImplementation. As a result, subsequent calls to functions defined in the original LevelOne contract (like getPrincipal) revert, since the proxy now points to a contract lacking those definitions.

Tools Used

Slither was used to identify this vulnerability.

Recommendations

To prevent unauthorized initialization and secure the upgradeable proxy pattern, it is strongly recommended to enforce access control on the initialize function of the LevelOne contract.

  1. Import OwnableUpgradeable from OpenZeppelin to enable access control:

import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

2.Call __Ownable_init() within the initialize function, before initializing UUPS:

function initialize(address _principal, uint256 _schoolFees, address _usdcAddress) public initializer {
if (_principal == address(0)) {
revert HH__ZeroAddress();
}
if (_schoolFees == 0) {
revert HH__ZeroValue();
}
if (_usdcAddress == address(0)) {
revert HH__ZeroAddress();
}
principal = _principal;
schoolFees = _schoolFees;
usdc = IERC20(_usdcAddress);
+ __Ownable_init(msg.sender);
__UUPSUpgradeable_init();Note: `__Ownable_init()` sets the caller `msg.sender` as the initial owner. If needed, you can pass a custom address instead of `msg.sender`.

Note: __Ownable_init() sets the caller msg.sender as the initial owner. If needed, you can pass a custom address instead of msg.sender.

Updates

Lead Judging Commences

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