Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Function `BidBeastsNFTMarket::takeHighestBid()` lacks a checks if the auction ended or not, allow seller to `takeHighestBid` after auction ends.

mozi

Function BidBeastsNFTMarket::takeHighestBid() lacks a checks if the auction ended or not, allow seller to takeHighestBid after auction ends.

Description

According to the natspec the purpose of BidBeastsNFTMarket::takeHighestBid() is "Allows the seller to accept the current highest bid before the auction ends." But, the function BidBeastsNFTMarket::takeHighestBid() lacks a checks if the auction ended or not.

/**
@> * @notice Allows the seller to accept the current highest bid before the auction ends.
*/
function takeHighestBid(uint256 tokenId) external isListed(tokenId) isSeller(tokenId, msg.sender) {
//@audit-issue there's no checks for if the auction ended or not
Bid storage bid = bids[tokenId];
require(bid.amount >= listings[tokenId].minPrice, "Highest bid is below min price");
​
_executeSale(tokenId);
}

Risk

Likelihood:

  • Whenever auction ends, seller still can call BidBeastsNFTMarket::takeHighestBid()

Impact:

  • Function logic does not match its purpose.

Proof of Concept

Copy and paste the PoC in BidBeastMarketPlaceTest.t.sol

function test_sellerStillCanTakeHighestBidAfterAuctionEnd() public {
_mintNFT();
_listNFT();
​
vm.prank(BIDDER_1);
market.placeBid{value: MIN_PRICE + 1}(TOKEN_ID);
​
// auction end + 10 seconds
vm.warp(block.timestamp + 910);
// seller still can call takeHighestBid
vm.prank(SELLER);
market.takeHighestBid(TOKEN_ID);
}

the log:

[PASS] test_sellerStillCanTakeHighestBidAfterAuctionEnd() (gas: 307892)
Traces:
[390130] BidBeastsNFTMarketTest::test_sellerStillCanTakeHighestBidAfterAuctionEnd()
├─ [0] VM::startPrank(ECRecover: [0x0000000000000000000000000000000000000001])
│ └─ ← [Return]
├─ [74067] BidBeasts::mint(SHA-256: [0x0000000000000000000000000000000000000002])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: SHA-256: [0x0000000000000000000000000000000000000002], tokenId: 0)
│ ├─ emit BidBeastsMinted(to: SHA-256: [0x0000000000000000000000000000000000000002], tokenId: 0)
│ └─ ← [Return] 0
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [25508] BidBeasts::approve(BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0)
│ ├─ emit Approval(owner: SHA-256: [0x0000000000000000000000000000000000000002], approved: BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], tokenId: 0)
│ └─ ← [Stop]
├─ [128588] BidBeastsNFTMarket::listNFT(0, 1000000000000000000 [1e18], 5000000000000000000 [5e18])
│ ├─ [1094] BidBeasts::ownerOf(0) [staticcall]
│ │ └─ ← [Return] SHA-256: [0x0000000000000000000000000000000000000002]
│ ├─ [29510] BidBeasts::transferFrom(SHA-256: [0x0000000000000000000000000000000000000002], BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0)
│ │ ├─ emit Transfer(from: SHA-256: [0x0000000000000000000000000000000000000002], to: BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], tokenId: 0)
│ │ └─ ← [Stop]
│ ├─ emit NftListed(tokenId: 0, seller: SHA-256: [0x0000000000000000000000000000000000000002], minPrice: 1000000000000000000 [1e18], buyNowPrice: 5000000000000000000 [5e18])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(RIPEMD-160: [0x0000000000000000000000000000000000000003])
│ └─ ← [Return]
├─ [72627] BidBeastsNFTMarket::placeBid{value: 1000000000000000001}(0)
│ ├─ emit AuctionSettled(tokenId: 0, winner: RIPEMD-160: [0x0000000000000000000000000000000000000003], seller: SHA-256: [0x0000000000000000000000000000000000000002], price: 1000000000000000001 [1e18])
│ ├─ emit AuctionExtended(tokenId: 0, newDeadline: 901)
│ ├─ emit BidPlaced(tokenId: 0, bidder: RIPEMD-160: [0x0000000000000000000000000000000000000003], amount: 1000000000000000001 [1e18])
│ └─ ← [Stop]
├─ [0] VM::warp(911)
│ └─ ← [Return]
├─ [0] VM::prank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [63269] BidBeastsNFTMarket::takeHighestBid(0)
│ ├─ [26871] BidBeasts::transferFrom(BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], RIPEMD-160: [0x0000000000000000000000000000000000000003], 0)
│ │ ├─ emit Transfer(from: BidBeastsNFTMarket: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: RIPEMD-160: [0x0000000000000000000000000000000000000003], tokenId: 0)
│ │ └─ ← [Stop]
│ ├─ [60] PRECOMPILES::sha256{value: 950000000000000001}(0x)
│ │ └─ ← [Return] 0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
│ ├─ emit AuctionSettled(tokenId: 0, winner: RIPEMD-160: [0x0000000000000000000000000000000000000003], seller: SHA-256: [0x0000000000000000000000000000000000000002], price: 1000000000000000001 [1e18])
│ └─ ← [Stop]
└─ ← [Stop]

Recommended Mitigation

Checks the auction deadline as it should be

/**
* @notice Allows the seller to accept the current highest bid before the auction ends.
*/
function takeHighestBid(uint256 tokenId) external isListed(tokenId) isSeller(tokenId, msg.sender) {
Bid storage bid = bids[tokenId];
+ require(block.timestamp < listings[tokenId].auctionEnd, "Auction is ended");
require(bid.amount >= listings[tokenId].minPrice, "Highest bid is below min price");
​
_executeSale(tokenId);
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

mozi Submitter
about 1 month ago
cryptoghost Lead Judge
about 1 month ago
cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.