Dria

Swan
NFTHardhat
21,000 USDC
View results
Submission Details
Severity: medium
Invalid

NFT Ownership Not Transferred During Purchase in Swan Protocol

Relevant Context

The Swan protocol allows users to list and purchase NFTs through buyer agents. When an NFT is purchased, it should be fully transferred to the buyer including both the token and ownership rights.

The SwanAsset contract inherits from both ERC721 and Ownable, meaning NFTs have separate token ownership and contract ownership. The contract ownership controls administrative rights.

Finding Description

In the Swan.purchase() function, when an NFT is purchased, only the ERC721 token is transferred to the buyer via transferFrom(), but the contract ownership is not transferred:

// Only transfers the token
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);

This creates a discrepancy where the buyer owns the NFT token but the seller retains contract ownership and administrative control over the NFT contract.

The issue becomes particularly problematic given the TODO comment indicating plans to use asset.owner() instead of seller in the future. This would mean the protocol would check contract ownership rather than token ownership, breaking core functionality.

Impact Explanation

Medium. While currently the contract ownership retention doesn't directly impact core functionality, it:

  1. Will break core protocol functionality if the planned change to use asset.owner() is implemented

  2. Creates an inconsistent ownership model that could confuse users and integrations

Likelihood Explanation

High. This issue affects every NFT purchase transaction in the protocol and will definitely cause problems if the planned changes are implemented.

Proof of Concept

  1. Alice creates and lists an NFT through Swan.list()

  2. Bob purchases the NFT through Swan.purchase()

  3. The NFT token is transferred to Bob

  4. Alice still retains contract ownership via Ownable

  5. If the protocol is updated to use asset.owner(), Bob won't be able to interact with the NFT despite being the token owner

Recommendation

Modify the purchase() function to transfer both token and contract ownership:

function purchase(address _asset) external {
// ... existing checks ...
// Transfer token ownership
SwanAsset(_asset).transferFrom(listing.seller, address(this), 1);
SwanAsset(_asset).transferFrom(address(this), listing.buyer, 1);
// Transfer contract ownership
SwanAsset(_asset).transferOwnership(listing.buyer);
// ... rest of the function ...
}

This ensures the buyer receives full control of the NFT, maintaining consistency between token and contract ownership.

Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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