code-423n4 / 2023-10-party-findings

6 stars 4 forks source link

If the contract was bid on before the NFT was gifted to the contract, lastBid will not be totalContributions #303

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-party/blob/main/contracts/crowdfund/AuctionCrowdfund.sol#L33-L73

Vulnerability details

Summary

In the finalize function of the AuctionCrowdfund contract, when the contract gets NFT and lastBid_ == 0, it is considered that NFT is gifted to the contract and everyone who contributed wins.
But if the contract was bid before the NFT was gifted to the contract, then since lastBid_ ! = 0, only the user who contributed at the beginning will win.

Proof of Concept

AuctionCrowdfund.sol#L33-L73

            function finalize(
        FixedGovernanceOpts memory governanceOpts,
        ProposalStorage.ProposalEngineOpts memory proposalEngineOpts
    ) external onlyDelegateCall returns (Party party_) {
        // Check that the auction is still active and has not passed the `expiry` time.
        CrowdfundLifecycle lc = getCrowdfundLifecycle();
        if (lc != CrowdfundLifecycle.Active && lc != CrowdfundLifecycle.Expired) {
            revert WrongLifecycleError(lc);
        }

        // Finalize the auction if it is not already finalized.
        uint96 lastBid_ = lastBid;
        _finalizeAuction(lc, market, auctionId, lastBid_);

        IERC721 nftContract_ = nftContract;
        uint256 nftTokenId_ = nftTokenId;
        // Are we now in possession of the NFT?
        if (nftContract_.safeOwnerOf(nftTokenId_) == address(this) && lastBid_ != 0) {
            // If we placed a bid before then consider it won for that price.
            // Create a governance party around the NFT.
            party_ = _createParty(
                governanceOpts,
                proposalEngineOpts,
                false,
                nftContract_,
                nftTokenId_
            );
            emit Won(lastBid_, party_);
        } else {
            // Otherwise we lost the auction or the NFT was gifted to us.
            // Clear `lastBid` so `_getFinalPrice()` is 0 and people can redeem their
            // full contributions when they burn their participation NFTs.
            lastBid = 0;
            emit Lost();
        }
        _bidStatus = AuctionCrowdfundStatus.Finalized;

        // Notify third-party platforms that the crowdfund NFT metadata has
        // updated for all tokens.
        emit BatchMetadataUpdate(0, type(uint256).max);
    }

Tools Used

Manual analysis + in-house tool

Recommended Mitigation Steps

Whether or not NFT is free to get should be determined using whether the contract balance is greater than totalContributions.

Assessed type

Other

ydspa commented 11 months ago

Invalid: OOS

c4-pre-sort commented 11 months ago

ydspa marked the issue as insufficient quality report

c4-judge commented 11 months ago

gzeon-c4 marked the issue as unsatisfactory: Out of scope