005.md

Cuddly Cream Jellyfish

High

Underlying implementation contracts left open to initialization by attackers in NounsAuctionHouseV3

Summary

Proxy implementation contracts along the inheritance chain could be left uninitialized, and vulnerable to initialization by attackers due to missing _disableInitializers() call in NounsAuctionHouseV3 contract.

Vulnerability Details

According to openzeppelin's documentation and comment suggestions, an uninitialized contract can be taken over by an attacker, and this applies to both the proxy and its implementation.

NounsAuctionHouseV3 inherits the proxy contract PausableUpgradeable which makes it vulnerable to uninitialization as it does not disable further initialization of the underlying implementation contracts.

Further notes on this topic include:

1. https://forum.openzeppelin.com/t/what-does-disableinitializers-function-mean/28730/2
2. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/14
3. https://docs.openzeppelin.com/contracts/5.x/api/proxy#Initializable-_disableInitializers--

Impact

Underlying proxy implementation contract takeover by attacker.

Resolution

Adding the following line to it's constructor prevents initialization of the implementations contract itself, adding extra protection from attackers:

    constructor(INounsToken _nouns, address _weth, uint256 _duration) initializer {
        nouns = _nouns;
        weth = _weth;
        duration = _duration; 

        _disableInitializers();                      // @audit-fix: disables implementation initialization
    }