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--;
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);
}
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
Alice lists NFT #1 at 10 USDC.
Bob approves unlimited USDC and submits buy(1).
Alice sees Bob's pending tx, front-runs with updatePrice(1, 4000e6).
Bob's buy(1) reads the new price — 4,000 USDC — and pays it.
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();
usdc.mint(bob, 5000e6);
vm.prank(bob);
usdc.approve(address(nftDealers), type(uint256).max);
vm.prank(alice);
nftDealers.updatePrice(1, 4000e6);
uint256 bobBalBefore = usdc.balanceOf(bob);
vm.prank(bob);
nftDealers.buy(1);
uint256 bobPaid = bobBalBefore - usdc.balanceOf(bob);
assertEq(bobPaid, 4000e6);
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);
}