Dria

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

Double royalty payment vulnerability in Swan Contract's relist function

Summary

The Swan contract contains a serious issue where sellers are charged royalty fees multiple times when relisting an asset. This happens because the relist function incorrectly calls transferRoyalties, causing sellers to pay royalties each time they relist an asset, rather than only on the initial listing.

Vulnerability Details

The issue can be found in the relist function of the Swan contract:

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

// File: contracts/swan/Swan.sol#L197-L255
function relist(address _asset, address _buyer, uint256 _price) external {
// ... existing checks ...
listings[_asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
assetsPerBuyerRound[_buyer][round].push(_asset);
// This line causes the double payment vulnerability
transferRoyalties(listings[_asset]); // @audit-issue Duplicate royalty payment
emit AssetRelisted(msg.sender, _buyer, _asset, _price);
}

The transferRoyalties function is called both during initial listing and relisting, causing duplicate royalty payments:

// File: contracts/swan/Swan.sol#L258-L272
function transferRoyalties(AssetListing storage asset) internal {
// calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
// first, Swan receives the entire fee from seller
token.transferFrom(asset.seller, address(this), buyerFee);
// send the buyer's portion to them
token.transfer(asset.buyer, buyerFee - driaFee);
// then it sends the remaining to Swan owner
token.transfer(owner(), driaFee);
}

Impact

Take a look at the POC test, it can be seen that clearly that sellers pay duplicate royalties, leading to unintended financial losses.

describe("POC: Royalty Double-Spending in Relist", () => {
let pocSeller: HardhatEthersSigner;
let pocBuyer: HardhatEthersSigner;
let pocBuyerAgent: BuyerAgent;
let assetAddress: string;
// Add constants
const ROYALTY_FEE = 1; // 1% royalty fee
const PRICE = parseEther("1.0");
// First, run the deployment test to ensure contracts are set up
it("should deploy contracts first", async function() {
// deploy token to be able to deploy swan
const supply = parseEther("1000");
token = await deployTokenFixture(dria, supply);
expect(await token.balanceOf(dria.address)).to.eq(supply);
const currentTime = (await ethers.provider.getBlock("latest").then((block) => block?.timestamp)) as bigint;
MARKET_PARAMETERS.timestamp = currentTime;
({ swan, registry, coordinator } = await deploySwanFixture(
dria,
token,
STAKES,
FEES,
MARKET_PARAMETERS,
ORACLE_PARAMETERS
));
expect(await swan.owner()).to.eq(dria.address);
expect(await swan.isOperator(dria.address)).to.be.true;
});
it("demonstrates double royalty payment on relist", async function() {
// Get signers - use different indices to avoid conflicts
[, , , , pocSeller, pocBuyer] = await ethers.getSigners();
// Fund the seller with enough tokens for multiple royalty payments
await token.connect(dria).transfer(pocSeller.address, parseEther("10.0"));
await token.connect(pocSeller).approve(await swan.getAddress(), parseEther("10.0"));
// Create a buyer agent
const tx = await swan.connect(pocBuyer).createBuyer(
"Test Buyer",
"Test Description",
ROYALTY_FEE,
parseEther("10.0")
);
const receipt = await tx.wait();
// Get the buyer agent address from the event
const event = receipt?.logs.find(
log => log.topics[0] === ethers.id("BuyerCreated(address,address)")
);
if (!event) throw new Error("BuyerCreated event not found");
const buyerAgentAddress = ethers.AbiCoder.defaultAbiCoder().decode(
['address'],
event.topics[2]
)[0];
// Get the BuyerAgent contract instance
pocBuyerAgent = await ethers.getContractAt("BuyerAgent", buyerAgentAddress) as BuyerAgent;
// Calculate expected royalty amounts
const royaltyAmount = (PRICE * BigInt(ROYALTY_FEE)) / 100n;
const platformAmount = (royaltyAmount * BigInt(MARKET_PARAMETERS.platformFee)) / 100n;
const buyerAmount = royaltyAmount - platformAmount;
// Get initial balances
const initialSellerBalance = await token.balanceOf(pocSeller.address);
const initialBuyerBalance = await token.balanceOf(await pocBuyerAgent.getAddress());
const initialPlatformBalance = await token.balanceOf(await swan.owner());
// First listing
await swan.connect(pocSeller).list(
"Test Asset",
"TEST",
ethers.encodeBytes32String("test"),
PRICE,
await pocBuyerAgent.getAddress()
);
// Get balances after first listing
const afterListSellerBalance = await token.balanceOf(pocSeller.address);
const afterListBuyerBalance = await token.balanceOf(await pocBuyerAgent.getAddress());
const afterListPlatformBalance = await token.balanceOf(await swan.owner());
// Verify first royalty payment
expect(initialSellerBalance - afterListSellerBalance).to.equal(royaltyAmount);
expect(afterListBuyerBalance - initialBuyerBalance).to.equal(buyerAmount);
expect(afterListPlatformBalance - initialPlatformBalance).to.equal(platformAmount);
// Get the listed asset address
const listedAssets = await swan.getListedAssets(await pocBuyerAgent.getAddress(), 0);
assetAddress = listedAssets[0];
// Advance time to next round
await time.increase(
MARKET_PARAMETERS.withdrawInterval +
MARKET_PARAMETERS.buyInterval +
MARKET_PARAMETERS.sellInterval
);
// Relist the asset
await swan.connect(pocSeller).relist(
assetAddress,
await pocBuyerAgent.getAddress(),
PRICE
);
// Get final balances
const finalSellerBalance = await token.balanceOf(pocSeller.address);
const finalBuyerBalance = await token.balanceOf(await pocBuyerAgent.getAddress());
const finalPlatformBalance = await token.balanceOf(await swan.owner());
// Verify double payment occurred
expect(initialSellerBalance - finalSellerBalance).to.equal(royaltyAmount * 2n);
expect(finalBuyerBalance - initialBuyerBalance).to.equal(buyerAmount * 2n);
expect(finalPlatformBalance - initialPlatformBalance).to.equal(platformAmount * 2n);
// Log the financial impact
console.log("Financial Impact of Double Royalty Payment:");
console.log("Seller lost extra tokens:", ethers.formatEther(royaltyAmount));
console.log("Buyer gained extra tokens:", ethers.formatEther(buyerAmount));
console.log("Platform gained extra tokens:", ethers.formatEther(platformAmount));
});
});

Test Results:

Financial Impact of Double Royalty Payment:
Seller lost extra tokens: 0.01
Buyer gained extra tokens: 0.0099
Platform gained extra tokens: 0.0001

The test confirms that for a 1 ETH asset:
Sellers lose 0.01 ETH in extra royalties per relist
Buyer agents receive 0.0099 ETH extra
Platform receives 0.0001 ETH extra

Tools Used

Manual code review
Hardhat testing framework
Chai assertion library
Ethers.js

Recommendations

Remove the royalty transfer from the relist function:

function relist(address _asset, address _buyer, uint256 _price) external {
// ... existing checks ...
listings[_asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
assetsPerBuyerRound[_buyer][round].push(_asset);
// Remove transferRoyalties call
emit AssetRelisted(msg.sender, _buyer, _asset, _price);
}

Alternative: If royalties on relisting are intended, implement a clear documentation and warning system to inform sellers about this cost.

Consider implementing a tracking mechanism to ensure royalties are only paid once per asset if that's the intended behavior.

Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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