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

Useless transfer if `marketParameters.platformFee == 100` resulting in a loss of fees paid by the `Swan` protocol

4th05

Relevant GitHub Links

https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol#L268

https://github.com/Cyfrin/2024-10-swan-dria/blob/main/contracts/swan/Swan.sol#L129

Summary

The protocol establish the presence of a platformFee for the service it provides that is a percentage of the buyerFee on every listed asset. Therefore, this value is always <=100.
However in case platformFee == 100 the protocol would pay fees for a useless transfer.

Vulnerability Details

In case platformFee == 100 we have that buyerFee == driaFee making the buyerFee-driaFee transfer useless because of 0 amount. However, there is no condition in the function to prevent this to happen.

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
// this allows only one approval from the seller's side
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

The amount of fees lost by the protocol for all the useless transactions. This amount depends on the number of listed assets when the platformFee == 100 condition is verified.
Although this is not likely it could happen because market parameters may change over time and we cannot say that there won't be platformFee == 100 considering a rerquirement in the Swanmanager::setMarketParameters stating _marketParameters.platformFee <= 100.

Tools Used

Manual review

Recommendations

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
// this allows only one approval from the seller's side
token.transferFrom(asset.seller, address(this), buyerFee);
​
+ if (buyerFee==driaFee){
+ token.transfer(owner(), driaFee);
+ } else {
// 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);
+ }
}
Updates

Lead Judging Commences

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