Skip to content
This repository has been archived by the owner on Nov 26, 2023. It is now read-only.

0x52 - Users can bypass Player royalties on EIP2981 compatible markets by selling clubs as a whole #293

Open
sherlock-admin opened this issue May 5, 2023 · 8 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

0x52

medium

Users can bypass Player royalties on EIP2981 compatible markets by selling clubs as a whole

Summary

Players have a royalty built in but clubs do not. This allows bulk sale of players via clubs to bypass the fee when selling players.

Vulnerability Detail

FootiumPlayer.sol#L16-L23

contract FootiumPlayer is
    ERC721Upgradeable,
    AccessControlUpgradeable,
    ERC2981Upgradeable,
    PausableUpgradeable,
    ReentrancyGuardUpgradeable,
    OwnableUpgradeable
{

FootiumPlayer implements the EIP2981 standard which creates fees when buy/selling the players.

FootiumClub.sol#L15-L21

contract FootiumClub is
    ERC721Upgradeable,
    AccessControlUpgradeable,
    PausableUpgradeable,
    ReentrancyGuardUpgradeable,
    OwnableUpgradeable
{

FootiumClub on the other hand never implements this standard. This allows users to sell players by selling their club to avoid any kind of fee on player sales.

Impact

Users can bypass fees on player sales by selling club instead

Code Snippet

FootiumClub.sol#L15-L21

Tool used

Manual Review

Recommendation

Implement EIP2981 on clubs as well

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels May 10, 2023
@logiclogue logiclogue added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels May 16, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 22, 2023
@thangtranth
Copy link

thangtranth commented May 22, 2023

Escalate for 10 USDC.

I believe this issue is invalid because when reading the codebase, it seems to be the design choice of the Footium protocol to not charge new owner of the club the royalty fee for player transfer. The reason is in theory, the players still remain in the same club and are not transferred to other club. Transfer royalty is only applicable when a player NFT is transferred from one club to another, not when a club NFT is sold to a new owner. It is consistent with how other football works.

Furthermore, there is no documentation that states or implies that buying a club NFT requires paying transfer royalty for every player NFT in the club. This leads me and many other Watsons to believe that this is not an issue.

The EIP2981 is not enforced, therefore if a user want to bypass the player royalties, there are many ways to do it so selling club as a whole is not necessary. For example listing in a NFT marketplace that does not support EIP2981, so no royalty is charged.

@sherlock-admin
Copy link
Contributor Author

sherlock-admin commented May 22, 2023

Escalate for 10 USDC.

I believe this issue is invalid because when reading the codebase, it seems to be the design choice of the Footium protocol to not charge new owner of the club the royalty fee for player transfer. The reason is in theory, the players still remain in the same club and are not transferred to other club. Transfer royalty is only applicable when a player NFT is transferred from one club to another, not when a club NFT is sold to a new owner. It is consistent with how other football works.

Furthermore, there is no documentation that states or implies that buying a club NFT requires paying transfer royalty for every player NFT in the club. This leads me and many other Watsons to believe that this is not an issue.

The EIP2981 is not enforced, therefore if a user want to bypass the player royalties, there are many ways to do it so selling club as a whole is not necessary. For example listing in a NFT marketplace that does not support EIP2981, so no royalty is charged.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label May 22, 2023
@0xRobocop
Copy link

Escalate for 10 USDC

This escalation is to contra-argument the escalation made by Watson thangtranth.

While it is correct that the issue over-estimated the "Users can bypass Player royalties", and that this can be done in an easier way with a marketplace that does not enforce royalties via EIP2981 as mentioned in the watson's escalation.

There exists an inconsistency in that players NFTs implement the EIP2981 whereas that club NFTs do not, so marketplaces that do respect the EIP-2981 won't pay the royalties for the club NFTs, and this can be seen as a loss of "yield" for the developers of the footium protocol.

The argument that this seemed to be the intended behavior (not charging royalties for clubs) is subjective, as it could have easily been missed by the developers. And given that the sponsor confirmed the issue, it seems that this was the case.

@sherlock-admin
Copy link
Contributor Author

Escalate for 10 USDC

This escalation is to contra-argument the escalation made by Watson thangtranth.

While it is correct that the issue over-estimated the "Users can bypass Player royalties", and that this can be done in an easier way with a marketplace that does not enforce royalties via EIP2981 as mentioned in the watson's escalation.

There exists an inconsistency in that players NFTs implement the EIP2981 whereas that club NFTs do not, so marketplaces that do respect the EIP-2981 won't pay the royalties for the club NFTs, and this can be seen as a loss of "yield" for the developers of the footium protocol.

The argument that this seemed to be the intended behavior (not charging royalties for clubs) is subjective, as it could have easily been missed by the developers. And given that the sponsor confirmed the issue, it seems that this was the case.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@logiclogue
Copy link

To clarify here from the protocol designer, it's correct that we don't intend for clubs to pay for the royalties for all players on transfer. Instead, we anticipate that the club will subjectively be valued in terms of the players that belong to the club. As a result, clubs should implement the EIP2981 standard so that on club sale royalties will be paid back to the protocol developers

@ctf-sec
Copy link
Collaborator

ctf-sec commented May 23, 2023

0xRobocop escalation is valid, a valid medium for this submission

@hrishibhat
Copy link

Escalation accepted

Valid medium
Given that the clubs should implement eip2981 to pay the protocol royalties on sale, considering this issue a valid medium

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Jun 3, 2023
@sherlock-admin
Copy link
Contributor Author

Escalation accepted

Valid medium
Given that the clubs should implement eip2981 to pay the protocol royalties on sale, considering this issue a valid medium

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

6 participants