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

Approval Exploit in `transferRoyalties` could cause transaction reverts

iampukar

Summary

The transferRoyalties function within the Swan contract is responsible for handling royalty payments when an asset is listed or relisted. This function calculates the necessary fees and transfers tokens from the asset seller to the Swan contract, subsequently distributing portions to the buyer and the Swan owner.

It performs the following operations:

// Calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
​
// Transfer royalties from seller to Swan
token.transferFrom(asset.seller, address(this), buyerFee);
​
// Distribute fees to buyer and Swan owner
token.transfer(asset.buyer, buyerFee - driaFee);
token.transfer(owner(), driaFee);

The function relies on the ERC20 transferFrom method to transfer buyerFee tokens from the seller to the Swan contract. This operation assumes that the seller has previously approved and continues to approve the Swan contract to spend at least buyerFee tokens on their behalf. If the allowance is revoked, the transferFrom call will fail, causing the entire transaction to revert.

Vulnerability Details

  • The seller holds an ERC20 token balance sufficient to cover buyerFee.

  • The seller has revoked the allowance for the Swan contract to transfer buyerFee tokens on their behalf.

  • A missing approval allowance check means that the transferFrom call within transferRoyalties will fail.

Impact

The failure of transferFrom causes the entire transaction, including the asset listing or relisting, to revert. This prevents the asset from being successfully listed and halts the royalty transfer process.

Tools Used

Manual Review

Recommendations

Before attempting the transferFrom operation, the transferRoyalties function should explicitly check that the seller has set sufficient allowances and not revoked them. This can provide clearer error messages and prevent unexpected transaction reverts.

function transferRoyalties(AssetListing storage asset) internal {
// Calculate fees
uint256 buyerFee = (asset.price * asset.royaltyFee) / 100;
uint256 driaFee = (buyerFee * getCurrentMarketParameters().platformFee) / 100;
​
// Check allowance
uint256 allowance = token.allowance(asset.seller, address(this));
if (allowance < buyerFee) {
revert InsufficientAllowance(asset.seller, buyerFee, allowance);
}
​
// Transfer royalties from seller to Swan
token.transferFrom(asset.seller, address(this), buyerFee);
​
// Distribute fees to buyer and Swan owner
token.transfer(asset.buyer, buyerFee - driaFee);
token.transfer(owner(), driaFee);
}
​
// Define a new error for clarity
error InsufficientAllowance(address seller, uint256 required, uint256 available);
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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.