Awesome

Summary

In January the Bonfida team found a critical vulnerability in the Metaplex auction smart contract which is used for most of Solana's NFT marketplaces, which exposed the funds of the Metaplex auction program to the risk of being stolen. This document is a short write-up explaining the mechanism behind the exploit and how it was fixed.

Exploit

When using the place_bid instruction, users had little constraint over what bidder_pot_token account they supplied (although it had to be owned by the auction). This means that they could supply a bidder_pot_token account which was already in use for this auction (for instance the one of another bidder). The funds for both bids could therefore be merged into the same token account. Then, the cancel_bid instruction would empty the entire token account into the supplied destination token account, thus enabling one user to steal the bid from another user (depending on the context, the funds are stolen from the bidder themselves or the auction seller).

Fix

The fix consisted of requiring the bidder pot token account to be uninitialized on bidder pot account creation (i.e. the first bid), with the added security that the token account is a PDA uniquely tied to the bidder pot account.

Detailed explanation

How does on-chain bidding even work?

You can think of Solana as a big key-value database in which you can store any data you need for your program. Elements of this database are called accounts. The Metaplex auction logic requires two Solana accounts to bid:

• Bidder pot: This is the Solana account containing the information about the auction, the bidder, the bid etc...
• Bidder pot token account: This is the Solana account serving as an escrow for the bid. The cancel bid instruction empties the content of this account into the user's wallet.

pub struct BidderPot {
    /// Points at actual pot that is a token account
    pub bidder_pot: Pubkey,
    /// Originating bidder account
    pub bidder_act: Pubkey,
    /// Auction metadata account
    pub auction_act: Pubkey,
    /// emptied or not
    pub emptied: bool,
}

As we can see above the bidder pot contains the public key of the bidder pot token account (bidder_pot field). The main issue came from the fact that the program was relying on the user to create the bidder pot token account before invoking the place bid instruction. Let's look at the code to see why the checks were not sufficient.

Original code

The original code can be found in the old.rs file. The instruction only performed the following checks:

• L111 the program checks the owner of the account is the SPL token program. This is to prevent an attacker from passing a fake token account (i.e. same data but different owner)

• L202 the program checks the account is initialized

• L203 the program checks that the token account owner is the auction

• L207 the program checks the token account has no delegate

• L211 the program checks the token account has no close authority

These checks do not prevent an attacker from passing a bidder pot token account already in use by someone else. Let's look at the following example:

1. Alice places a bid:

   • Alice has to create the bidder pot token account before invoking the place_bid instruction of Metaplex. She assigns the auction as the owner of this account.
   • Alice invokes the place_bid instruction, all checks pass, and the bid is placed. Her bid funds are then stored in the above bidder pot token account.

2. Bob steals Alice's bid:

   • Bob sees Alice's bid.
   • Bob invokes the place_bid instruction and passes Alice's bidder pot token account. All the checks pass by because Alice's bidder pot token account passed the checks when she executed a place_bid, and nothing has changed to affect the results of those checks. Bob's bid funds are therefore added to Alice's bidder pot token account
   • Bob now has a valid bidder account which shares Alice's bidder pot token account. Bob invokes a cancel_bid instruction, which dumps the entirety of the bidder pot token account's balance into one of his own token accounts. Bob has therefore acquired Alice's bid in addition to recovering his own.

Using a very simple script to go through all active bidder pot accounts in the Metaplex extended ecosystem (any program using the Metaplex auction program would have been a collateral target), then performing the above, an attacker could then steal all bids.

New code

Commit 94cfc600d570cfc6b880d73ee64206a83b572ca3 fixed the vulnerability by requiring the bidder pot token to be uninitialized and a PDA.

Bounty

Metaplex bounty program can be found here, for this vulnerability Metaplex offered a bounty of 100k.

TL;DR

Whenever an account is passed by the user it should be considered unsafe and should be checked. In many cases using PDAs will reduce the surface of attack of your program as it guarantees that the initialization of the account is made by the program itself. We strongly recommend reading Neodyme's blog to learn more about common security pitfalls in Solana programming.