NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: medium

`buy()` has no slippage protection — seller can front-run `updatePrice` to overcharge the buyer

Author Revealed upon completion

Root + Impact

Description

The buy() function only takes a listing ID. It reads the price from storage at execution time, with no way for the buyer to say "I'm willing to pay at most X." A malicious seller can watch the mempool, see an incoming buy() transaction, and front-run it with updatePrice() to jack up the price right before the buyer's transaction lands.

If the buyer approved a large amount of USDC (infinite approvals are standard practice in DeFi), they silently pay the inflated price with no recourse.

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");
​
activeListingsCounter--;
​
// @> price is read from storage — seller can change it between buyer's submit and execution
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;
// @> no maxPrice parameter — buyer is completely unprotected
emit NFT_Dealers_Sold(msg.sender, listing.price);
}

Risk

Likelihood: Front-running is practical on all major EVM chains. The seller just needs to submit updatePrice with higher gas. MEV bots make this even easier.

Impact: The buyer pays up to type(uint32).max (~4,294 USDC) or the full extent of their USDC approval — whichever is lower. For users with infinite approvals, this can mean losing thousands of dollars on what was supposed to be a cheap purchase.

Proof of Concept

  1. Alice lists NFT #1 at 10 USDC.

  2. Bob approves unlimited USDC and submits buy(1).

  3. Alice sees Bob's pending tx, front-runs with updatePrice(1, 4000e6).

  4. Bob's buy(1) reads the new price — 4,000 USDC — and pays it.

  5. Bob spent 400x what he expected. No warning, no revert.

function testM03_FrontRunPriceInflation() public {
usdc.mint(alice, 20e6);
vm.startPrank(alice);
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
nftDealers.list(1, 10e6);
vm.stopPrank();
​
// Bob approves unlimited (common practice)
usdc.mint(bob, 5000e6);
vm.prank(bob);
usdc.approve(address(nftDealers), type(uint256).max);
​
// Alice front-runs: 10 USDC → 4000 USDC
vm.prank(alice);
nftDealers.updatePrice(1, 4000e6);
​
// Bob's buy goes through at the inflated price
uint256 bobBalBefore = usdc.balanceOf(bob);
vm.prank(bob);
nftDealers.buy(1);
​
uint256 bobPaid = bobBalBefore - usdc.balanceOf(bob);
assertEq(bobPaid, 4000e6); // paid 4000 instead of 10
assertEq(nftDealers.ownerOf(1), bob);
}

Recommended Mitigation

Add a maxPrice parameter so the buyer can cap what they're willing to pay. This is standard slippage protection — the same pattern used by Uniswap, OpenSea, and virtually every DeFi protocol.

- function buy(uint256 _listingId) external payable {
+ function buy(uint256 _listingId, uint256 _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");
​
activeListingsCounter--;
​
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;
emit NFT_Dealers_Sold(msg.sender, listing.price);
}

Support

FAQs

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

Give us feedback!