Submission Details
Impact:
Likelihood:
collectUsdcFromSelling Transfers Fees to address(this)
Author Revealed upon completion
Root + Impact
collectUsdcFromSelling attempts to separate fees from seller proceeds, but the fee transfer destination is address(this) — the contract itself. This is a no-op: the contract transfers USDC to itself, which changes nothing. The full amountToSeller (which already had fees subtracted) is then sent to the seller. The result is that totalFeesCollected increments in storage, but the actual USDC backing those fees was never retained — it remains in the general contract balance and is effectively double-counted.
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
...
totalFeesCollected += fees;
usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
}
Risk
Likelihood:
This executes on every single
collectUsdcFromSellingcall without exception — 100% of fee accounting is broken from day oneNo special condition required; the bug is structural
Impact:
withdrawFees()will attempt to transfertotalFeesCollectedto the owner, but if the contract balance has been depleted by other sellers collecting proceeds, the withdrawal will revert or overdrawOwner can never reliably withdraw the correct fee amount — either getting too much (draining others' funds) or reverting when balance is insufficient
Proof of Concept
// SPDX-License-Identifier: UNLICENSED
import {Test, console} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
totalSupply += amount;
}
function transfer(address to, uint256 amount) external returns (bool) {
require(balanceOf[msg.sender] >= amount, "insufficient balance");
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
function approve(address spender, uint256 amount) external returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
require(allowance[from][msg.sender] >= amount, "allowance exceeded");
require(balanceOf[from] >= amount, "insufficient balance");
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
}
contract CollectFeesBug_PoC is Test {
NFTDealers public marketplace;
MockUSDC public usdc;
address public owner = makeAddr("owner");
address public alice = makeAddr("alice"); // Minter/Seller 1
address public bob = makeAddr("bob"); // Buyer/Seller 2
address public charlie = makeAddr("charlie"); // Buyer 2
uint256 constant LOCK_AMOUNT = 20e6; // 20 USDC collateral
uint256 constant LISTING_PRICE = 1000e6; /
uint256 constant EXPECTED_FEE_1PCT = 10e6;
function setUp() public {
usdc = new MockUSDC();
// Deploy marketplace
marketplace = new NFTDealers(
owner,
address(usdc),
"Test Collection",
"TEST",
"ipfs://base/",
LOCK_AMOUNT
);
// Fund users with USDC
usdc.mint(alice, 10000e6);
usdc.mint(bob, 10000e6);
usdc.mint(charlie, 10000e6);
// Whitelist users and reveal collection
vm.startPrank(owner);
marketplace.whitelistWallet(alice);
marketplace.whitelistWallet(bob);
marketplace.whitelistWallet(charlie);
marketplace.revealCollection();
vm.stopPrank();
}
function test_FeeTransferIsNoOp() public {
console.log("=== PoC: Fee Transfer to address(this) is No-Op ===\n");
vm.startPrank(alice);
usdc.approve(address(marketplace), LOCK_AMOUNT);
marketplace.mintNft();
uint256 tokenId = 1;
vm.stopPrank();
console.log("✓ Alice minted tokenId=%d, locked %s USDC collateral",
tokenId, _fmt(LOCK_AMOUNT));
vm.startPrank(alice);
marketplace.list(tokenId, uint32(LISTING_PRICE));
vm.stopPrank();
console.log("✓ Alice listed tokenId=%d for %s USDC\n",
tokenId, _fmt(LISTING_PRICE));
vm.startPrank(bob);
usdc.approve(address(marketplace), LISTING_PRICE);
marketplace.buy(tokenId);
vm.stopPrank();
uint256 contractBalanceAfterBuy = usdc.balanceOf(address(marketplace));
console.log("✓ Bob bought tokenId=%d", tokenId);
console.log(" Contract USDC balance after buy: %s USDC\n",
_fmt(contractBalanceAfterBuy));
uint256 balanceBeforeCollect = usdc.balanceOf(address(marketplace));
uint256 feesBefore = marketplace.totalFeesCollected();
vm.startPrank(alice);
marketplace.collectUsdcFromSelling(tokenId);
vm.stopPrank();
uint256 balanceAfterCollect = usdc.balanceOf(address(marketplace));
uint256 feesAfter = marketplace.totalFeesCollected();
uint256 aliceBalance = usdc.balanceOf(alice);
console.log("✓ Alice called collectUsdcFromSelling()");
console.log(" Contract balance before: %s USDC", _fmt(balanceBeforeCollect));
console.log(" Contract balance after: %s USDC", _fmt(balanceAfterCollect));
console.log(" Balance change: %s USDC (should be -%s for fees)",
_fmt(balanceBeforeCollect - balanceAfterCollect),
_fmt(EXPECTED_FEE_1PCT));
console.log(" totalFeesCollected before: %s USDC", _fmt(feesBefore));
console.log(" totalFeesCollected after: %s USDC", _fmt(feesAfter));
console.log(" Alice received: %s USDC (proceeds + collateral)\n",
_fmt(aliceBalance - LOCK_AMOUNT)); // minus her original collateral
// ─────────────────────────────────────────────────────────────
// ASSERTION 1: Fee accounting is INCORRECT
// ─────────────────────────────────────────────────────────────
assertEq(feesAfter - feesBefore, EXPECTED_FEE_1PCT, "Fees should be recorded");
// BUG DEMONSTRATION: Balance only decreased by amountToSeller, not fees
uint256 expectedBalanceDrop = LISTING_PRICE - EXPECTED_FEE_1PCT + LOCK_AMOUNT;
assertEq(
balanceBeforeCollect - balanceAfterCollect,
expectedBalanceDrop,
"BUG: Contract balance dropped by seller proceeds ONLY, fees not segregated"
);
console.log("🚨 BUG CONFIRMED:");
console.log(" - totalFeesCollected increased by %s USDC ✓", _fmt(EXPECTED_FEE_1PCT));
console.log(" - But contract balance did NOT retain those fees ✗");
console.log(" - Fees exist in accounting but not in actual segregated balance\n");
vm.startPrank(bob);
usdc.approve(address(marketplace), LOCK_AMOUNT);
marketplace.mintNft();
uint256 tokenId2 = 2;
marketplace.list(tokenId2, uint32(LISTING_PRICE));
vm.stopPrank();
vm.startPrank(charlie);
usdc.approve(address(marketplace), LISTING_PRICE);
marketplace.buy(tokenId2);
vm.stopPrank();
vm.startPrank(bob);
marketplace.collectUsdcFromSelling(tokenId2);
vm.stopPrank();
uint256 finalContractBalance = usdc.balanceOf(address(marketplace));
uint256 totalFeesRecorded = marketplace.totalFeesCollected();
console.log("✓ Second sale completed (Bob→Charlie)");
console.log(" Final contract balance: %s USDC", _fmt(finalContractBalance));
console.log(" totalFeesCollected: %s USDC", _fmt(totalFeesRecorded));
console.log(" Expected fees (2 sales): %s USDC", _fmt(EXPECTED_FEE_1PCT * 2));
// ─────────────────────────────────────────────────────────────
// ASSERTION 2: Owner cannot reliably withdraw fees
// ─────────────────────────────────────────────────────────────
// If owner tries to withdraw NOW:
vm.startPrank(owner);
uint256 ownerBalanceBefore = usdc.balanceOf(owner);
// This will SUCCEED but withdraw MORE than actual fees if balance allows,
// or REVERT if balance is insufficient
try marketplace.withdrawFees() {
uint256 ownerBalanceAfter = usdc.balanceOf(owner);
uint256 withdrawn = ownerBalanceAfter - ownerBalanceBefore;
console.log("\n⚠️ withdrawFees() succeeded, withdrew: %s USDC", _fmt(withdrawn));
if (withdrawn > totalFeesRecorded) {
console.log("🚨 CRITICAL: Owner withdrew MORE than recorded fees!");
console.log(" This drains funds that belong to sellers/collateral!");
}
} catch Error(string memory reason) {
console.log("\n🚨 CRITICAL: withdrawFees() reverted: %s", reason);
console.log(" Owner fees are permanently locked!");
}
vm.stopPrank();
console.log("\n=== FINAL STATE ===");
console.log("Contract USDC balance: %s", _fmt(usdc.balanceOf(address(marketplace))));
console.log("totalFeesCollected: %s (accounting)", _fmt(marketplace.totalFeesCollected()));
console.log("\n💥 CONCLUSION: Fee accounting is decoupled from actual token flows.");
console.log(" The contract cannot guarantee correct fee withdrawals.");
}
function _fmt(uint256 amount) internal pure returns (string memory) {
uint256 whole = amount / 1e6;
uint256 decimals = amount % 1e6;
return string.concat(
vm.toString(whole), ".",
decimals < 100000 ? "0" : "",
decimals < 10000 ? "0" : "",
decimals < 1000 ? "0" : "",
decimals < 100 ? "0" : "",
decimals < 10 ? "0" : "",
vm.toString(decimals)
);
}
}
Recommended Mitigation
- function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
...
totalFeesCollected += fees;
- usdc.safeTransfer(address(this), fees);
usdc.safeTransfer(msg.sender, amountToSeller);
+ // fees remain in contract balance, tracked by totalFeesCollected
}
+
Rewards breakdown
nSLOC
253This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.