juan - Roles within any `Edition` contract can never be granted/revoked

[sherlock-admin3]

juan

high

Roles within any Edition contract can never be granted/revoked

Summary

Edition.grantRoles and Edition.revokeRoles are uncallable by the owner of an Edition, leading to lost protocol functionality.

The protocol intends the following functionality for the EDITION_MANAGER_ROLE:

EDITION_MANAGER_ROLE (Restricted) => On an Edition, this role can: 1) Publish a new work with any desired configuration (publish). This is the only way to create new works after the Edition is created. 2) Mint promotional copies of any work (promoMint). There are no limitations on this action aside from the work's supply cap and minting period. 3) Set the Edition's ERC2981 royalty receiver (setRoyaltyTarget). This is the only way to change the royalty receiver for the Edition. 4) Grant or revoke any role to/from any address (grantRole, revokeRole).

However 2/4 of these functionalities (3 and 4) will not be possible due to the bug.

Vulnerability Detail

When each Edition contract is initialised by the TitlesCore contract, the TitlesContract is given the EDITION_MANAGER_ROLE. This is shown here:

function initialize(
        FeeManager feeManager_,
        TitlesGraph graph_,
        address owner_,
        address controller_,
        Metadata calldata metadata_
    ) external initializer {
        // Other code omitted

        _grantRoles(controller_, EDITION_MANAGER_ROLE); // TitlesCore is granted the manager role

        // Other code omitted
    }

Hence, the following two functions can only be called by the TitlesCore contract. However, the TitlesCore contract does not have any logic that can call Edition.grantRoles or Edition.revokeRoles.

function grantRoles(address user_, uint256 roles_)
        public
        payable
        override
        onlyRoles(EDITION_MANAGER_ROLE)
    {
        _grantRoles(user_, roles_);
    }

    /// @inheritdoc OwnableRoles
    function revokeRoles(address user_, uint256 roles_)
        public
        payable
        override
        onlyRoles(EDITION_MANAGER_ROLE)
    {
        _removeRoles(user_, roles_);
    }

As a result, these functions are uncallable. This means that the owner of an edition will not be able to grant roles such as the EDITION_MINTER_ROLE to anybody. This means that only the owner will be allowed to call promoMint(), while the project intends for the owner to be able to allow others to call promoMint via the EDITION_MINTER_ROLE as stated here:

EDITION_MINTER_ROLE (Restricted) => On an Edition, this role can: 1) Mint promotional copies of any work (promoMint). There are no limitations on this action aside from the work's supply >cap and minting period.

Also, they won't be able to grant EDITION_MANAGER_ROLE to anybody else, which renders the setRoyaltyTarget() function useless after initialisation since it also has the onlyRoles(EDITION_MANAGER_ROLE) modifier.

In total: the functionality of grantRole(), revokeRole(), setRoyaltyTarget()are completely blocked due to this logical bug.

Impact

Edition owners will not be able to grant or revoke roles on an edition. As a result, a large amount of core protocol functionality is lost- user is never being able to grant various important roles to anybody.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/c9d16782a7d3c15c7a759f22c9e0552d5e777ed7/wallflower-contract-v2/src/editions/Edition.sol#L423-L440

Tool used

Manual Review

Recommendation

Change the modifier to onlyRolesOrOwner rather than onlyRoles.

Duplicate of #148

[ccashwell]

This is expected, the EDITION_MANAGER_ROLE holder (TitlesCore) will be upgraded at a later point to leverage this functionality for a planned feature. In any case, it's not a vulnerability.

[WangSecurity]

I agree that this should be duplicated with #148

[0xjuaan]

@WangSecurity pls confirm if this has been added as a dupe of #148

the other dupes have the 'medium' tag but this one doesn't yet