code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

AuctionHouse.sol#createBid() - Malicious user can block stuff other bidders to unfairly win the auction #590

Open c4-bot-2 opened 11 months ago

c4-bot-2 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/AuctionHouse.sol#L171-L200 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/AuctionHouse.sol#L336-L414

Vulnerability details

Impact

AuctionHouse implements a special timeBuffer , which is used to extend an auction by it, if a bid is created during said timeBuffer .

function createBid(uint256 verbId, address bidder) external payable override nonReentrant {
        IAuctionHouse.Auction memory _auction = auction;

        //require bidder is valid address
        require(bidder != address(0), "Bidder cannot be zero address");
        require(_auction.verbId == verbId, "Verb not up for auction");
        //slither-disable-next-line timestamp
        require(block.timestamp < _auction.endTime, "Auction expired");
        require(msg.value >= reservePrice, "Must send at least reservePrice");
        require(
            msg.value >= _auction.amount + ((_auction.amount * minBidIncrementPercentage) / 100),
            "Must send more than last bid by minBidIncrementPercentage amount"
        );

        address payable lastBidder = _auction.bidder;

        auction.amount = msg.value;
        auction.bidder = payable(bidder);

        // Extend the auction if the bid was received within `timeBuffer` of the auction end time
        bool extended = _auction.endTime - block.timestamp < timeBuffer;
        if (extended) auction.endTime = _auction.endTime = block.timestamp + timeBuffer;

        // Refund the last bidder, if applicable
        if (lastBidder != address(0)) _safeTransferETHWithFallback(lastBidder, _auction.amount);

        emit AuctionBid(_auction.verbId, bidder, msg.sender, msg.value, extended);

        if (extended) emit AuctionExtended(_auction.verbId, _auction.endTime);
    }

If the timeBuffer is 10 minutes, if someone bids in the last 10 minutes of the auction, the auction endTime will be extended by 10 minutes.

This is implemented to give a fair chance for all bidders to big on the auction, if the auction is especially competitive.

The protocol also plans to deploy to Base and Optimism.

Transactions on Optimism and Base are much cheaper than Ethereum, which opens up a rare attack vector: block stuffing.

A malicious user stuffs an entire block with dummy transactions that consume the entire block gas limit, which is 30m gas on Optimism or ~7$ worth of gas, basically, it’s extremely cheap.

If a user is the current bidder and there is only the timeBuffer left (15 minutes for example), he can block stuff for 15 minutes straight to win the auction.

Let’s make some quick calculations:

timeBuffer == 15 minutes , this is the value used in the tests, so we’ll use it here.

15 minutes = 900 seconds

Optimism creates a block every 2 seconds, so in 15 minutes, there will be 450 blocks created on Optimism.

1 block = 30m gas, so 450 blocks will have 13.5b gas, which is ~3000 $.

Considering that the protocol auctions off NFT’s which can be sold for huge amounts (https://www.masoative.com/post/most-expensive-nft), spending an extra 3000$ to potentially win hundreds of thousands, even millions is a pretty lucrative deal.

The attack can get cheaper or more expensive, depending on the timebuffer .

Proof of Concept

Example:

  1. An art piece is created through the CultureIndex
  2. It gets dropped and the auction for it begins.
  3. Users continue bidding, until a whale becomes the top bidder.
  4. Only 15 minutes are left until the end of the auction (timeBuffer) and the whale wants to 100% win it.
  5. He block stuffs 450 blocks, spending an extra 3000$ to make sure he wins the auction.
  6. After the auction ends, he calls settleAuction which transfers the verb token to him, after which he can sell to make a profit.

The likelihood of this scenario is not uncommon at all, there have been several instances of block stuffing attacks where malicious actors used the technique to win lotteries/auctions etc. Considering how expensive some NFT’s are, the scenario is 100% possible.

You can read a bit more about block stuffing and how it was used to win a lottery here. https://medium.com/hackernoon/the-anatomy-of-a-block-stuffing-attack-a488698732ae

You can input the gas values here to see the current prices. https://www.cryptoneur.xyz/en/gas-fees-calculator

Tools Used

Manual Review

Recommended Mitigation Steps

I’m not sure what to recommend as there is no way to stop this sort of attack.

One possible way is to make timeBuffer much larger to make a block stuffing attack more expensive.

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #112

c4-judge commented 10 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

MarioPoneder marked the issue as grade-c

0xdeth commented 10 months ago

Hi @MarioPoneder,

This issue is not a duplicate of #112.

112 uses the extend functionality to win the bid, while this issue uses block stuffing to unfairly win the bid.

I have showcased how easily and relatively cheaply someone can block stuff for enough time to win an art piece that can easily cover the attackers cost, during the block stuff.

I have also showcased how block stuffing was used in the past in the exact same way: the attacker used block stuffing to win a lottery by simply blocking other users from using it. This issue is exactly the same, but inside an auction, not a lottery.

Because of this I believe this is a valid Medium severity issue.

Cheers and thanks for your time.

PlamenTSV commented 10 months ago

I believe this issue is valid, https://github.com/code-423n4/2023-12-revolutionprotocol-findings/issues/27 as it's dupe, as they cover a completely different attack concept that is easily economically achievable.

MarioPoneder commented 10 months ago

Thank you for your comment!

I appreciate you raising awareness about this issue again. @rocketman-21 However, as it's basically an attack on the network and rather relating to future deployments, QA still seems most appropriate.

All the best!

c4-judge commented 10 months ago

MarioPoneder marked the issue as grade-b

radeveth commented 10 months ago

Hey, @MarioPoneder!

I strongly believe that this issue should not be considered an attack on the network. As explained in all reports related to this vulnerability (#590, #730, #27), a malicious actor can win an auction in a way that is unfavorable to the protocol by employing block stuffing.

Here is a similar issue that was considered a valid medium severity issue in a previous Code4rena contest: https://github.com/code-423n4/2023-05-venus-findings/issues/525

Therefore, this vulnerability, which has a significant impact, should undoubtedly be mitigated by the protocol. Some recommendations for mitigation include:

Have a good one!

MarioPoneder commented 10 months ago

Thank you for your comment and I appreciate the mitigation recommendations!

notbozho commented 10 months ago

Hey, @MarioPoneder!

I believe that this issue (and his dups #730 , #27 ) should be classified as Medium Severity. The wardens above explain in detail why this vulnerability has a significant impact on the Revolution Protocol.

Cheers.

MarioPoneder commented 10 months ago
  1. The timeBuffer is set unreasonably low in this example and can be configured by the owner/DAO even after deployment, therefore nothing to worry.
  2. The assumed lenght of block stuffing attacks is unreasonably high. The one provided in the example with 175 seconds is already extraordinary.