024.md

Soft White Peacock

Medium

Missing disableinitializers() in NounsAuctionHouseV2 and NounsAuctionHouseV3 constructors can lead to malicious takeover of the implementation contract

Summary

An uninitialized contract can be taken over by an attacker. This applies to both a proxy and its implementation contract, which may impact the proxy. The concern arises due to the usage of a proxy upgradeable contract without calling disableInitializers in the constructor of the logic contract. The NounsAuctionHouseV2 and NounsAuctionHouseV3 contracts lack the _disableInitializers() function call in its constructor. This oversight introduces a severe risk, allowing the attackers to initialize the implementation contract itself and take control of critical state variables within the contract.

Root Cause

In the NounsAuctionHouseV2.sol:76 is missing the disableinitializers(). In the NounsAuctionHouseV3.sol:86 is missing the disableinitializers().

Internal pre-conditions

• The NounsAuctionHouseV2.sol:76 and NounsAuctionHouseV3.sol:86 contracts use OpenZeppelin's upgradeable base contracts (PausableUpgradeable, ReentrancyGuardUpgradeable, OwnableUpgradeable).
• The contracts implement its own constructor.

External pre-conditions

• An attacker has access to deploy contracts using this vulnerable implementation.
• There aren't additional checks or restrictions on who can call the initialize function after deployment.

Attack Path

1. The attacker identifies the vulnerable implementation contract (NounsAuctionHouseV2 or NounsAuctionHouseV3).
2. Call the initialize() function directly on the implementation contract.
3. Manipulate initialization parameters to take control of critical state variables within the contract.

Impact

The attackers can initialize the implementation contract itself and take the control of the implementation contract.

Mitigation

Call disableInitializers: Include a call to disableInitializers in the constructors of the logic contracts (here and here) as recommended by OpenZeppelin here.