Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0xCTTTTTTT - Attacker will control auction mechanism through proxy reinitialization for all Nouns bidders #164

Open
sherlock-admin3 opened this issue Nov 30, 2024 · 0 comments

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Nov 30, 2024

0xCTTTTTTT

Medium

Attacker will control auction mechanism through proxy reinitialization for all Nouns bidders

Summary

The missing _disableInitializers() in implementation contracts will cause a complete manipulation of auction parameters for all Nouns participants as attackers will frontrun proxy upgrades to reinitialize storage with malicious values

Root Cause

In NounsAuctionHouseV2.sol#L72-L76 and NounsAuctionHouseV3.sol#L82-L86 there is a missing _disableInitializers() call in the constructor which allows the implementation contract to be reinitialized after upgrade

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

  1. Admin calls upgrade() to deploy new implementation contract
  2. Attacker calls initialize() with malicious parameters:
    • Low reservePrice
    • High timeBuffer
    • High minBidIncrementPercentage
    • Malicious streamEscrow address
  3. Admin's initialization fails as attacker already initialized contract

Impact

Impact

The protocol suffers from critical parameter manipulation due to unprotected initialization. The attacker can:

  1. DoS auction participation by setting extreme bid requirements:

    • minBidIncrementPercentage = 255 (max uint8) forces 255% higher bids
    • Normal users cannot participate due to excessive bid requirements
  2. Manipulate auction timing:

    • timeBuffer = 86400 (1 day) extends auction by full day on each bid
    • User funds get trapped in extended auctions
    • Unfair advantage for timing manipulation
  3. Break protocol economics:

    • reservePrice = 0 allows free auction wins
    • immediateTreasuryBPs = 0 blocks treasury revenue
    • streamLengthInTicks = 1 minimizes streaming period
    • Protocol loses control over economic parameters

PoC

function test_InitializerAttack() public {
    console.log("Initial Reserve Price:", auctionHouse.reservePrice());
    console.log("Initial Time Buffer:", auctionHouse.timeBuffer());
    console.log("Initial Min Bid Increment:", auctionHouse.minBidIncrementPercentage());

    vm.startPrank(attacker);
    auctionHouse.initialize(
        0, // reservePrice set to 0
        1 days, // timeBuffer set to maximum
        255 // minBidIncrementPercentage set to maximum
    );
    vm.stopPrank();

    assertEq(auctionHouse.reservePrice(), 0, "Reserve price should be 0");
    assertEq(auctionHouse.timeBuffer(), 1 days, "Time buffer should be 1 day");
    assertEq(auctionHouse.minBidIncrementPercentage(), 255, "Min bid increment should be 255");

    console.log("After Attack Reserve Price:", auctionHouse.reservePrice());
    console.log("After Attack Time Buffer:", auctionHouse.timeBuffer());
    console.log("After Attack Min Bid Increment:", auctionHouse.minBidIncrementPercentage());
}

Mitigation

Add _disableInitializers() in the constructor to prevent re-initialization after deployment.

@sherlock-admin4 sherlock-admin4 changed the title Expert Tangerine Bobcat - Attacker will control auction mechanism through proxy reinitialization for all Nouns bidders 0xCTTTTTTT - Attacker will control auction mechanism through proxy reinitialization for all Nouns bidders Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant