GroupBuy may not purchase the lowest available offer

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tessera/blob/f37a11407da2af844bbfe868e1422e3665a5f8e4/src/modules/GroupBuy.sol#L160-L161

Vulnerability details

Impact

A GroupBuy can whitelist a list of tokenid to buy, but there are there are no way to ensure the lowest available offer is purchased. This allow a seller of any of the target NFTs able to extract more value from the GroupBuy.

Proof of Concept

Beside NFT seller monitoring the Tessera can backrun the last contribution to buy their own NFT (even if it is not the cheapest one, but above the pool budget), they can also extract more value if users decided to bid higher in the GroupBuy:

1. A GroupBuy pool is created to buy any Punk at 100 eth with 100 millions Rae
2. Bob (with other users) bought 90 millions Rae with 180 eth by bidding higher to have higher chance to be included in the GroupBuy
3. They know that the GroupBuy will only happen at the minimum reserve price
4. Alice bought the remaining 10 millions Rae to push up the minimum reserve price
5. Alice atomically purchased her Punk listed at 200 eth after buying the remaining Rae

    function testPurchaseRevertExpensive() public {
        // setup
        testContributeSuccess();
        uint256 prevTotalContribution = totalContribution;
        uint256 nftPrice = minValue * 2;
        // execute
        vm.expectRevert();
        _purchase(
            address(this),
            currentId,
            address(punksBuyer),
            address(punks),
            punkId,
            nftPrice,
            purchaseOrder,
            purchaseProof
        );
        // expect
        assertEq(isForSale, false);
        assertEq(seller, address(punksBuyer));
        assertEq(minValue, 0);
        assertEq(success, true);
        assertEq(punks.punkIndexToAddress(punkId), vault);
        assertEq(totalContribution, prevTotalContribution - nftPrice);
        assertEq(filledQuantity, totalSupply);
    }

Tools Used

Foundry

Recommended Mitigation Steps

Since it is impossible to monitor the available bid onchain, a solution can be allowing anyone to suggest purchase candidates within a certain time period and execute the lowest priced one.

[c4-judge]

HickupHH3 marked the issue as satisfactory

[c4-judge]

HickupHH3 marked the issue as primary issue

[c4-judge]

HickupHH3 marked the issue as selected for report

[HickupHH3]

Selecting as best report because the write-up is more detailed.

[trust1995]

I believe the report is technically correct but it is a known issue and a scenario taken into consideration. PartyDAO which is a very similar project explained how they have made peace with the issue: "The issue with this is that a motivated actor could still buy the original listing and create a new one on an allowed target then trigger the party to buy() that one instead at a higher price, pocketing the difference. We have made peace with this issue and accepted the risk, but the team is open to new solutions."

Ultimately you could say that the users were satisfied with buying the NFT at the agreed-upon bid price, so charging them less is a nice-to-have but not a requirement or a bug.

[gzeoneth]

I believe the report is technically correct but it is a known issue and a scenario taken into consideration. PartyDAO which is a very similar project explained how they have made peace with the issue:

"The issue with this is that a motivated actor could still buy the original listing and create a new one on an allowed target then trigger the party to buy() that one instead at a higher price, pocketing the difference. We have made peace with this issue and accepted the risk, but the team is open to new solutions."

Ultimately you could say that the users were satisfied with buying the NFT at the agreed-upon bid price, so charging them less is a nice-to-have but not a requirement or a bug.

I would say if the protocol almost guarantee max slippage to users it is certainly an issue. The difference between this and PartyBid is that this can allowlist multiple NFT, which allow seller actually have an incentive to compete the price lower, but this exploit allow any seller to extract maximum value. Individual seller can not move the price up themselves and guarantee fill if the lowest priced offer is purchased because other seller will undercut.

In country, PartyBid only bid on an single NFT where you have different dynamics.

[trust1995]

I believe it is pretty clear that the bid is intended to be for potentially different NFTs of similar value in the eyes of bidder. If NFTs are valued different they would be held in different groupbuys for the price to represent correctly.

[gzeoneth]

I believe it is pretty clear that the bid is intended to be for potentially different NFTs of similar value in the eyes of bidder. If NFTs are valued different they would be held in different groupbuys for the price to represent correctly.

Hmm ya, they should be similar and you should buy the cheapest one, which is exactly my point. The current design allow any one of the sellers inflate the price and guarantee fill.

I think there are many case in which people just ape bid higher to give some buffer and also it cost gas to modify bid, and they trusted the protocol to price it fairly (e.g. minimum reserve price, will only buy the cheapest listing). It's entirely possible to allow the protocol to buy a cheaper listing by using an optimistic design where you allow people to commit purchase order and execute it with a slight delay.

[trust1995]

I agree, but I also think you would agree that this is sounding more like a protocol feature than a HIGH risk finding. Protocol can do better to reduce slippage, but it does not have to defend user's reckless bidding habits.

[gzeoneth]

I agree, but I also think you would agree that this is sounding more like a protocol feature than a HIGH risk finding. Protocol can do better to reduce slippage, but it does not have to defend user's reckless bidding habits.

Even without bidding higher I still don't think the protocol should buy a more expensive listing when it is possible to buy a lower priced one, when it is reasonably possible.

I also can't totally agree with the protocol does not have to defend these bidding behaviors, it is common, and it is like saying a perpetual future protocol don't have to defend against price manipulation because their user should not use leverage.

[gzeoneth]

I would also add that the protocol didn't work very well unless everyone bid higher. Imagine everyone pooled $1000 at $1 per Rae but now you need $1010 to buy the NFT. So what now, someone contribute $10 more? No, you need everyone to contribute again at $1.01. So the normal behavior would be to bid higher to avoid the inconvenience (also note that if they need to pool up to buy a NFT, the few dollars of gas cost is probably a significant cost)

[c4-sponsor]

stevennevins marked the issue as disagree with severity

[0x0aa0]

@trust1995 @gzeoneth Really great discussion here. We do recognize this potential scenario but as @trust1995 mentioned

PartyDAO which is a very similar project explained how they have made peace with the issue

We have made peace with the issue in the same way and as @trust1995 also mentions

The bid is intended to be for potentially different NFTs of similar value in the eyes of bidder. If NFTs are valued different they would be held in different groupbuys for the price to represent correctly.

Our view is that users will organize pools in this way to prevent severe slippage.

[HickupHH3]

@0x0aa0 @stevennevins agree with downgrading the severity to either medium or QA. Leaning towards the latter because I agree with this sentiment:

Ultimately you could say that the users were satisfied with buying the NFT at the agreed-upon bid price, so charging them less is a nice-to-have but not a requirement or a bug.

Any objections?

[stevennevins]

yeah agreed on QA here

[c4-judge]

Duplicate of https://github.com/code-423n4/2022-12-tessera-findings/issues/22

[c4-judge]

HickupHH3 marked the issue as not selected for report