NFTTrader Exploit — Reentrancy via `editCounterPart()` During Swap Settlement

Source & credit. Exploit reproduction, trace data, and analysis adapted from DeFiHackLabs by SunWeb3Sec — an open registry of reproduced on-chain exploits. Standalone Foundry PoC and full write-up: 2023-12-NFTTrader_exp in the evm-hack-registry mirror. Upstream DeFiHackLabs PoC: src/test/…/NFTTrader_exp.sol.

Vulnerability classes: vuln/reentrancy/cross-function

One-line summary: A missing reentrancy guard in NFTTrader's closeSwapIntent() lets the swap creator hijack the onERC721Received callback (fired while settling their own NFT) to call editCounterPart() and re-point the swap's counterpart to an arbitrary victim — causing the contract to pull the victim's pre-approved NFTs to the attacker for free.

Reproduction: the PoC compiles & runs in an isolated Foundry project at this project folder. Full verbose trace: output.txt. Verified vulnerable source (deployed name BatchSwap): contracts_BatchSwap.sol.

Key info#

TL;DR#

NFTTrader is a peer-to-peer NFT swap escrow. A user creates a swap intent listing the NFTs they offer (nftsOne) and the NFTs they expect from a counterpart (nftsTwo), and names a counterpart in addressTwo. When closeSwapIntent() is called by the counterpart, the contract moves nftsOne from the creator to the counterpart, then moves nftsTwo from the counterpart back to the creator — a fair, atomic swap.

The settlement function has three fatal flaws that compose:

1. No reentrancy guard. closeSwapIntent() is not nonReentrant.
2. It re-reads addressTwo from storage in the second transfer loop instead of using a cached value (contracts_BatchSwap.sol:246-264).
3. editCounterPart() can mutate addressTwo at any time, by the creator, with no status check and no reentrancy guard (:346-349).

The attacker is both the creator (addressOne) and the counterpart (addressTwo) of their own swap. They list one of their own NFTs (a freshly-minted Uniswap-V3 position) in nftsOne, and a victim's NFT in nftsTwo. On closeSwapIntent():

• The first loop sends the attacker's own NFT from the contract... but in this attack the NFT in nftsOne is actually one the attacker still owns and that NFTTrader pulls via safeTransferFrom, which fires the attacker's onERC721Received.
• Inside that callback the attacker calls editCounterPart(swapId, victim) — flipping the swap's addressTwo from attacker to victim.
• The second loop then re-reads addressTwo (now = victim) and executes CloneX.safeTransferFrom(victim → addressOne) — pulling the victim's CloneX NFT to the attacker.

Because the victim had previously set NFTTrader as an approved operator for CloneX, the transfer succeeds. The attacker pays nothing for the victim's NFT. Repeating the swap once per victim NFT drains all 5 CloneX tokens (victim balance 5 → 0, attacker 0 → 5).

Background — what NFTTrader does#

NFTTrader's BatchSwap contract (source) is an escrow for two-sided NFT/ERC20 swaps:

• createSwapIntent (:141-194) — the creator (addressOne = msg.sender) registers a swap, naming a counterpart addressTwo and two asset lists: nftsOne (assets the creator escrows now) and nftsTwo (assets the counterpart will provide at close). Creator's nftsOne ERC721s are pulled into the contract immediately.
• closeSwapIntent (:197-270) — callable by addressTwo. Moves escrowed nftsOne from the contract to addressTwo, then moves nftsTwo from addressTwo to addressOne. This is the settlement.
• editCounterPart (:346-349) — lets the creator change who the counterpart is.
• whiteList — only whitelisted dapps (NFT contracts) can be swapped. Both CloneX (0x49cF…28B) and the Uniswap V3 Position NFT (0xC364…FE88) were whitelisted at the time of the attack (the trace shows both transfers passing the whiteList[...] require checks).

The economic model assumes that once addressTwo calls close and provides their nftsTwo, the swap is balanced: each side gives and receives. The exploit destroys that balance by changing who addressTwo is between the "give" and the "receive."

The vulnerable code#

1. closeSwapIntent caches addressTwo, transfers nftsOne (callback!), then re-reads addressTwo#

function closeSwapIntent(address _swapCreator, uint256 _swapId) payable public whenNotPaused {
    require(swapList[_swapCreator][swapMatch[_swapId]].status == swapStatus.Opened, "...");
    require(swapList[_swapCreator][swapMatch[_swapId]].addressTwo == msg.sender, "...not the interested counterpart");
    ...
    swapList[_swapCreator][swapMatch[_swapId]].addressTwo = msg.sender;   // L216: addressTwo = attacker
    swapList[_swapCreator][swapMatch[_swapId]].status = swapStatus.Closed; // L218

    //From Owner 1 to Owner 2  (transfer nftsOne to addressTwo)
    for(i=0; i<nftsOne[_swapId].length; i++) {
        ...
        else if(nftsOne[_swapId][i].typeStd == ERC721) {
            ERC721Interface(nftsOne[_swapId][i].dapp).safeTransferFrom(
                address(this), swapList[...].addressTwo, nftsOne[_swapId][i].tokenId[0], ...); // L227-228 ⚠️ fires onERC721Received on attacker
        }
        ...
    }
    ...
    //From Owner 2 to Owner 1  (transfer nftsTwo from addressTwo to addressOne)
    for(i=0; i<nftsTwo[_swapId].length; i++) {
        ...
        else if(nftsTwo[_swapId][i].typeStd == ERC721) {
            ERC721Interface(nftsTwo[_swapId][i].dapp).safeTransferFrom(
                swapList[...].addressTwo,   // ⚠️ L252: RE-READ from storage — now == victim
                swapList[...].addressOne,
                nftsTwo[_swapId][i].tokenId[0], ...);
        }
        ...
    }
}

Lines 197-270. Two problems:

• L227-228: transferring nftsOne via safeTransferFrom invokes onERC721Received on the recipient (the attacker, who is the current addressTwo) — an external call before the second loop runs, with no reentrancy guard.
• L252: the second loop reads swapList[...].addressTwo again from storage as the from of the outgoing transfer. It is not a cached local, so a reentrant write to addressTwo takes effect here.

2. editCounterPart — creator can re-point addressTwo at will, no guard, no status check#

function editCounterPart(uint256 _swapId, address payable _counterPart) public {
    require(msg.sender == swapList[msg.sender][swapMatch[_swapId]].addressOne, "Message sender must be the swap creator");
    swapList[msg.sender][swapMatch[_swapId]].addressTwo = _counterPart;
}

Lines 346-349. The only check is that the caller is the swap creator (addressOne). There is:

• no nonReentrant modifier,
• no whenNotPaused,
• no check that the swap is still Opened (it can be edited even while/after closing),
• no check that the swap is not currently being settled.

The attacker is the creator, so this require passes when called from inside the attacker's onERC721Received callback.

3. The attacker's callback weaponizes both#

From the PoC (test/NFTTrader_exp.sol:184-193):

function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data)
    external returns (bytes4) {
    // Flawed function. Lack of reentrancy protection
    NFTTrader.editCounterPart(swapId, victim);   // re-point addressTwo → victim, mid-settlement
    return this.onERC721Received.selector;
}

Root cause — why it was possible#

The settlement of a swap is not atomic with respect to the swap's own parameters. Between moving nftsOne out and moving nftsTwo in, control flow leaves the contract (via the ERC721 safeTransferFrom → onERC721Received hook on nftsOne), and during that window the swap's counterpart address can be mutated by the creator.

Four design decisions compose into a critical bug:

1. No reentrancy guard on closeSwapIntent. A nonReentrant modifier would have made the reentrant editCounterPart call revert, killing the attack.
2. addressTwo is re-read from storage in the second transfer loop (L252) rather than captured into an immutable local at the top of the function. The first loop and the access-control check (L199) saw addressTwo = attacker; the second loop sees whatever storage now holds.
3. editCounterPart is callable mid-settlement. It enforces only "caller is creator," not "swap is open and not currently executing." A require(status == Opened) plus a reentrancy guard would block it.
4. safeTransferFrom of the creator's own NFT hands control to the creator. In a self-swap (attacker = creator = counterpart), the nftsOne recipient is the attacker, so the attacker receives a callback during their own swap's settlement. (Even in a non-self swap, the standard ERC721 push hook is an attacker-controllable callback if the counterpart is a contract.)

The net effect: the attacker turns a balanced 1-for-1 swap (give my UniV3 NFT, get a CloneX NFT back from the same counterpart) into a one-sided theft — the contract pulls the victim's CloneX NFT because addressTwo was silently swapped to the victim after the access-control check already passed. The victim's standing setApprovalForAll(NFTTrader, true) is what makes the pull succeed.

Preconditions#

• Victim has approved NFTTrader as an operator for the target NFT collection (CloneX.isApprovedForAll(victim, NFTTrader) == true). The PoC asserts this at test/NFTTrader_exp.sol:120. This is the standard approval a user grants to use the platform; it is the single resource the attack consumes.
• Both NFT contracts are whitelisted in NFTTrader (whiteList[CloneX] and whiteList[UniV3PosNFT] are true). The trace shows both whiteList[...] requires passing.
• The attacker controls a whitelisted, callback-bearing NFT to list in nftsOne. Any safeTransferFrom-based ERC721 works; the PoC mints a fresh Uniswap-V3 position NFT (tokenId 625712) with 0.001 ETH (test/NFTTrader_exp.sol:98-112).
• Trivial capital. The attacker pays only swap fees (0.005 ETH per createSwapIntent / closeSwapIntent in the PoC) plus ~0.001 ETH to mint the bait NFT. No flash loan required.

Step-by-step attack walkthrough (per victim NFT)#

The PoC loops this sequence 5 times, once per victim CloneX tokenId. The numbers below are for the first iteration (swapId 10348, CloneX tokenId 6670), taken directly from output.txt. Subsequent iterations are identical with the next tokenId.

Final balances (from output.txt logs):

Victim CloneX balance before attack:    5
Exploiter CloneX balance before attack: 0
Victim CloneX balance after attack:     0
Exploiter CloneX balance after attack:  5

The trace confirms each editCounterPart storage write (e.g. the slot 0x003ee421…419d9 flips from the attacker address to the victim address) immediately followed by the CloneX::safeTransferFrom(Victim → ContractTest, 6670).

Profit / loss accounting#

The attacker spent only gas + protocol fees and walked away with the victim's entire CloneX collection. No assets of the attacker's own were lost — the bait UniV3 NFT was only self-transferred.

Diagrams#

Sequence of one swap iteration#

Swap state evolution within closeSwapIntent#

Where the guard should have been#

Remediation#

1. Add a reentrancy guard. Mark closeSwapIntent, createSwapIntent, cancelSwapIntent, and editCounterPart with OpenZeppelin's nonReentrant. This alone defeats the attack: the reentrant editCounterPart call would revert.
2. Cache the counterpart once and use the cached value. At the top of closeSwapIntent, read address counterpart = swapList[...].addressTwo; into a local and use that local for both loops and all access checks. Never re-read mutable swap state from storage between the two transfer phases (:252 is the offending re-read).
3. Forbid mutation of an in-flight / closed swap. editCounterPart (:346-349) must require(swap.status == swapStatus.Opened) and be nonReentrant, so it cannot be called during or after settlement.
4. Follow checks-effects-interactions and lock the swap during settlement. Set status to a transient Closing state (or a boolean lock) before any external transfer, and reject any state edits while that lock is held.
5. Prefer pull/transferFrom settlement or vet recipients. If callbacks must be tolerated, treat every safeTransferFrom as an untrusted external call and ensure no swap-defining state can change as a result of it.

NFTTrader's documented post-incident fix was to advise all users to revoke approvals to the vulnerable contract and to deploy a patched contract with reentrancy protection.

How to reproduce#

_shared/run_poc.sh 2023-12-NFTTrader_exp -vvvvv

• RPC: a mainnet archive endpoint is required (fork block 18,799,414, Dec 2023). foundry.toml uses an Infura archive endpoint; pruned public RPCs fail with header not found / missing trie node.
• Result: [PASS] testExploit().

Expected tail:

Ran 1 test for test/NFTTrader_exp.sol:ContractTest
[PASS] testExploit() (gas: 3094204)
Logs:
  Victim CloneX balance before attack: 5
  Exploiter CloneX balance before attack: 0
  Victim CloneX balance after attack: 0
  Exploiter CloneX balance after attack: 5

Suite result: ok. 1 passed; 0 failed; 0 skipped

References: SlowMist Hacked — https://hacked.slowmist.io/ (NFTTrader, Ethereum, ~$3M). Analyses: @AnciliaInc, @SlowMist_Team, @0xArhat (linked in the PoC header).

Sources & further analysis#

Reproductions & code

• Standalone PoC + full trace: 2023-12-NFTTrader_exp (evm-hack-registry mirror).
• Upstream DeFiHackLabs PoC: NFTTrader_exp.sol.

Alerts & third-party analyses

• DeFiHackLabs incident explorer: search "NFTTrader Exploit".
• Web3Sec X hacked database: search.
• Rekt leaderboard: search.
• Solodit incident search: search.

These dashboards index community alerts tweets, post-mortems, and independent write-ups. Reach them through the protocol name above to cross-check this reproduction against other analyses.