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

A single asset appears to be owned by multiple buyers

0xbrett8571

Summary

The Swan protocol's relisting mechanism fails to maintain proper asset ownership tracking, allowing assets to be counted multiple times across different buyers.

function relist(address _asset, address _buyer, uint256 _price) external {
// @audit-info Asset remains tracked under old buyer
(uint256 oldRound,,) = BuyerAgent(asset.buyer).getRoundPhase();
// @audit-info Adds to new buyer without cleaning old reference
assetsPerBuyerRound[_buyer][round].push(_asset);
// @audit-info Creates new listing without proper state cleanup
listings[_asset] = AssetListing({
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
}

Vulnerability Details

When relisting an asset, the asset gets added to assetsPerBuyerRound for the new buyer, but it's not removed from the previous buyer's assets array

Swan.sol#function list: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L160-L182
Swan.sol#function relist: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L237-L249
Swan.sol#function transferRoyalties: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L258-L261

function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
// @audit-info Missing zero address check for _buyer parameter
BuyerAgent buyer = BuyerAgent(_buyer);
// @audit-info No validation on _price - could be set to 0 allowing free assets
listings[asset] = AssetListing({
price: _price,
//...
});
}
​
function relist(address _asset, address _buyer, uint256 _price) external {
// @audit-info Asset remains in old buyer's array creating double counting
assetsPerBuyerRound[_buyer][round].push(_asset);
// @audit-info No check if asset was previously sold in old round
listings[_asset] = AssetListing({
status: AssetStatus.Listed,
//...
});
}
​
function transferRoyalties(AssetListing storage asset) internal {
// @audit-info Potential rounding errors in fee calculations
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
}

The relisting mechanism allows assets to appear in multiple buyer arrays simultaneously, breaking the core invariant of unique asset ownership.

Lack of minimum price validation could allow assets to be listed for free or near-zero amounts, potentially disrupting the market dynamics.

The double division in fee calculations can lead to rounding errors that accumulate over time, affecting protocol revenue distribution.

Proof of Concept:

// 1. Initial listing
swan.list("Asset1", "A1", "desc", 1 ether, buyerA);
​
// 2. Check initial state
assert(swan.getListedAssets(buyerA, round1).length == 1);
​
// 3. Relist to new buyer
swan.relist(asset1, buyerB, 2 ether);
​
// 4. Asset appears in both arrays
assert(swan.getListedAssets(buyerA, round1).length == 1); // Still shows
assert(swan.getListedAssets(buyerB, round2).length == 1); // Also shows
​
// 5. Total asset count exceeds actual assets
assert(totalAssets > uniqueAssets); // Invariant violation

Impact

  1. Breaks core protocol invariants around asset ownership

  2. Affects market statistics and constraints

  3. Could lead to economic implications in fee distribution

  4. Undermines the reliability of buyer agent performance metrics

Tools Used

Vs

Recommendations

The relist() function should:

  1. Remove the asset from the previous buyer's array

  2. Then add it to the new buyer's array

function relist(address _asset, address _buyer, uint256 _price) external {
AssetListing storage asset = listings[_asset];
+ // Remove from old buyer's tracking
+ removeAssetFromBuyerRound(asset.buyer, asset.round, _asset);
// Add to new buyer
assetsPerBuyerRound[_buyer][round].push(_asset);
+ // Update listing with clean state transition
listings[_asset] = AssetListing({
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
+ emit AssetRelistingUpdated(_asset, asset.buyer, _buyer);
}
​
+ function removeAssetFromBuyerRound(address buyer, uint256 round, address asset) internal {
+ address[] storage assets = assetsPerBuyerRound[buyer][round];
+ for (uint i = 0; i < assets.length; i++) {
+ if (assets[i] == asset) {
+ assets[i] = assets[assets.length - 1];
+ assets.pop();
+ break;
+ }
+ }
+ }
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.