Fix [M-1] Native tokens can get locked when bidding or buying from th…

…e marketplace
thirdweb-dev · Sep 26, 2023 · 0422600 · 0422600
1 parent 88ddc9e
commit 0422600
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 54 deletions.
diff --git a/contracts/prebuilts/marketplace/direct-listings/DirectListingsLogic.sol b/contracts/prebuilts/marketplace/direct-listings/DirectListingsLogic.sol
@@ -285,6 +285,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
         if (_currency == CurrencyTransferLib.NATIVE_TOKEN) {
             require(msg.value == targetTotalPrice, "Marketplace: msg.value must exactly be the total price.");
         } else {
+            require(msg.value == 0, "Marketplace: invalid native tokens sent.");
             _validateERC20BalAndAllowance(buyer, _currency, targetTotalPrice);
         }
 

diff --git a/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol b/contracts/prebuilts/marketplace/english-auctions/EnglishAuctionsLogic.sol
@@ -135,6 +135,10 @@ contract EnglishAuctionsLogic is IEnglishAuctions, ReentrancyGuard, ERC2771Conte
             "Marketplace: inactive auction."
         );
         require(_bidAmount != 0, "Marketplace: Bidding with zero amount.");
+        require(
+            _targetAuction.currency == CurrencyTransferLib.NATIVE_TOKEN || msg.value == 0,
+            "Marketplace: invalid native tokens sent."
+        );
 
         Bid memory newBid = Bid({ auctionId: _auctionId, bidder: _msgSender(), bidAmount: _bidAmount });
 

diff --git a/src/test/marketplace/DirectListings.t.sol b/src/test/marketplace/DirectListings.t.sol
@@ -1865,6 +1865,38 @@ contract MarketplaceDirectListingsTest is BaseTest, IExtension {
         vm.prank(_seller);
         listingId = DirectListingsLogic(marketplace).createListing(listingParams);
     }
+
+    function test_audit_native_tokens_locked() public {
+        (uint256 listingId, IDirectListings.Listing memory existingListing) = _setup_buyFromListing();
+
+        uint256[] memory tokenIds = new uint256[](1);
+        tokenIds[0] = existingListing.tokenId;
+
+        // Verify existing auction at `auctionId`
+        assertEq(existingListing.assetContract, address(erc721));
+
+        vm.warp(existingListing.startTimestamp);
+
+        // No ether is locked in contract
+        assertEq(marketplace.balance, 0);
+
+        // buy from listing
+        erc20.mint(buyer, 10 ether);
+        vm.deal(buyer, 1 ether);
+
+        vm.prank(seller);
+        DirectListingsLogic(marketplace).approveBuyerForListing(listingId, buyer, true);
+
+        vm.startPrank(buyer);
+        erc20.approve(marketplace, 10 ether);
+
+        vm.expectRevert("Marketplace: invalid native tokens sent.");
+        DirectListingsLogic(marketplace).buyFromListing{ value: 1 ether }(listingId, buyer, 1, address(erc20), 1 ether);
+        vm.stopPrank();
+
+        // 1 ether is temporary locked in contract
+        assertEq(marketplace.balance, 0 ether);
+    }
 }
 
 contract IssueC2_MarketplaceDirectListingsTest is BaseTest, IExtension {
@@ -2106,36 +2138,4 @@ contract IssueC2_MarketplaceDirectListingsTest is BaseTest, IExtension {
         // assertBalERC20Eq(address(erc20), buyer, 0);
         // assertBalERC20Eq(address(erc20), seller, totalPrice);
     }
-
-    function test_audit_native_tokens_locked() public {
-        (uint256 listingId, IDirectListings.Listing memory existingListing) = _setup_buyFromListing();
-
-        uint256[] memory tokenIds = new uint256[](1);
-        tokenIds[0] = existingListing.tokenId;
-
-        // Verify existing auction at `auctionId`
-        assertEq(existingListing.assetContract, address(erc721));
-
-        vm.warp(existingListing.startTimestamp);
-
-        // No ether is locked in contract
-        assertEq(marketplace.balance, 0);
-
-        // buy from listing
-        erc20.mint(buyer, 10 ether);
-        vm.deal(buyer, 1 ether);
-
-        vm.prank(seller);
-        DirectListingsLogic(marketplace).approveBuyerForListing(listingId, buyer, true);
-
-        vm.startPrank(buyer);
-        erc20.approve(marketplace, 10 ether);
-        DirectListingsLogic(marketplace).buyFromListing{ value: 1 ether }(listingId, buyer, 1, address(erc20), 1 ether);
-        vm.stopPrank();
-
-        assertIsOwnerERC721(address(erc721), buyer, tokenIds);
-
-        // 1 ether is temporary locked in contract
-        assertEq(marketplace.balance, 1 ether);
-    }
 }
diff --git a/src/test/marketplace/EnglishAuctions.t.sol b/src/test/marketplace/EnglishAuctions.t.sol
@@ -1916,31 +1916,13 @@ contract MarketplaceEnglishAuctionsTest is BaseTest, IExtension {
 
         vm.startPrank(buyer);
         erc20.approve(marketplace, 10 ether);
+
+        vm.expectRevert("Marketplace: invalid native tokens sent.");
         EnglishAuctionsLogic(marketplace).bidInAuction{ value: 1 ether }(auctionId, 10 ether);
         vm.stopPrank();
 
-        (address bidder, address currency, uint256 bidAmount) = EnglishAuctionsLogic(marketplace).getWinningBid(
-            auctionId
-        );
-
-        // Test consequent states.
-        // Seller is owner of token.
-        assertIsOwnerERC721(address(erc721), buyer, tokenIds);
-        assertEq(erc20.balanceOf(marketplace), 10 ether);
-        assertEq(erc20.balanceOf(buyer), 0);
-        assertEq(buyer, bidder);
-        assertEq(currency, address(erc20));
-        assertEq(bidAmount, 10 ether);
-
-        // collect auction payout
-        vm.prank(seller);
-        EnglishAuctionsLogic(marketplace).collectAuctionPayout(auctionId);
-
-        assertEq(erc20.balanceOf(marketplace), 0);
-        assertEq(erc20.balanceOf(seller), 10 ether);
-
-        // 1 ether is temporary locked in contract
-        assertEq(marketplace.balance, 1 ether);
+        // No ether is temporary locked in contract
+        assertEq(marketplace.balance, 0);
     }
 }