Summary
The Swan protocol's core fee distribution mechanism contains critical vulnerabilities in price validation and token transfer safety, potentially leading to system-wide economic exploitation.
function transferRoyalties(AssetListing storage asset) internal {
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
token.transferFrom(asset.seller, address(this), buyerFee);
token.transfer(asset.buyer, buyerFee - driaFee);
token.transfer(owner(), driaFee);
}
function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external {
listings[asset] = AssetListing({
price: _price,
royaltyFee: buyer.royaltyFee(),
});
}
Vulnerability Details
These functions don't properly track operation counts, which should increment atomically with successful operations.
list() function creates new assets and listings
purchase() function handles asset transfers and payments
relist() function allows relisting assets
Swan.sol#function List: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L160-L182
function list(string calldata _name, string calldata _symbol, bytes calldata _desc, uint256 _price, address _buyer)
external
{
BuyerAgent buyer = BuyerAgent(_buyer);
(uint256 round, BuyerAgent.Phase phase,) = buyer.getRoundPhase();
if (phase != BuyerAgent.Phase.Sell) {
revert BuyerAgent.InvalidPhase(phase, BuyerAgent.Phase.Sell);
}
if (getCurrentMarketParameters().maxAssetCount == assetsPerBuyerRound[_buyer][round].length) {
revert AssetLimitExceeded(getCurrentMarketParameters().maxAssetCount);
}
address asset = address(swanAssetFactory.deploy(_name, _symbol, _desc, msg.sender));
listings[asset] = AssetListing({
createdAt: block.timestamp,
royaltyFee: buyer.royaltyFee(),
price: _price,
seller: msg.sender,
status: AssetStatus.Listed,
buyer: _buyer,
round: round
});
}
Affect core protocol functionality around asset listing and trading mechanics. The combination of missing validations and access controls creates significant risk for market manipulation and economic attacks.
Swan.sol#function transferRoyalties: https://github.com/Cyfrin/2024-10-swan-dria/blob/c3f6f027ed51dd31f60b224506de2bc847243eb7/contracts/swan/Swan.sol#L258-L272
function transferRoyalties(AssetListing storage asset) internal {
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
token.transferFrom(asset.seller, address(this), buyerFee);
token.transfer(asset.buyer, buyerFee - driaFee);
token.transfer(owner(), driaFee);
}
For example
await swan.list("Test", "TST", "0x", MAX_UINT256, buyer.address);
const buyerFee = price.mul(royaltyFee).div(100);
const driaFee = buyerFee.mul(platformFee).div(100);
await swan.purchase(assetId);
Impact
Affects all asset listings and purchases
Disrupts fee distribution mechanism
Potential permanent loss of funds
Tools Used
Vs
Recommendations
function transferRoyalties(AssetListing storage asset) internal {
+ require(asset.price > 0 && asset.price <= MAX_PRICE, "Invalid price");
+ require(asset.royaltyFee <= MAX_ROYALTY, "Invalid royalty");
+ uint256 buyerFee = SafeMath.mul(asset.price, asset.royaltyFee) / 100;
+ uint256 driaFee = SafeMath.mul(buyerFee, getCurrentMarketParameters().platformFee) / 100;
+ require(token.balanceOf(asset.seller) >= buyerFee, "Insufficient balance");
+ require(token.allowance(asset.seller, address(this)) >= buyerFee, "Insufficient allowance");
- token.transferFrom(asset.seller, address(this), buyerFee);
+ SafeERC20.safeTransferFrom(token, asset.seller, address(this), buyerFee);
- token.transfer(asset.buyer, buyerFee - driaFee);
+ SafeERC20.safeTransfer(token, asset.buyer, buyerFee - driaFee);
- token.transfer(owner(), driaFee);
+ SafeERC20.safeTransfer(token, owner(), driaFee);
}
