sherlock-audit / 2024-04-titles-judging

12 stars 9 forks source link

0x486776 - There are some useless functions in `Edition.sol`. #196

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 6 months ago

0x486776

medium

There are some useless functions in Edition.sol.

Summary

Edition::grantRoles and Edition::promoMint functions are useless as there is no implementation logic to invoke them.

Vulnerability Detail

As seen at L427 of Edition::grantRoles, the caller must possess the EDITION_MANAGER_ROLE.

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

However, the EDITION_MANAGER_ROLE is solely assigned to TitlesCore.sol, which lacks the implementation logic to invoke Edition::grantRoles.

And as you can see at L329, Edition::promoMint can only be accessed by someone holding the EDITION_MANAGER_ROLE or EDITION_MINTER_ROLE.

    function promoMint(address[] calldata receivers_, uint256 tokenId_, bytes calldata data_)
        external
329     onlyOwnerOrRoles(EDITION_MANAGER_ROLE | EDITION_MINTER_ROLE)
    {
        for (uint256 i = 0; i < receivers_.length; i++) {
            _issue(receivers_[i], tokenId_, 1, data_);
        }
    }

However, since Edition::grantRoles is never invoked, there is no one who possesses the EDITION_MINTER_ROLE. Additionally, TitlesCore.sol that possesses EDITION_MANAGER_ROLE has no implementation logic to call Edition::promoMint.

Impact

The promotinoal purpose can't be achieved.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L327-L334

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

Tool used

Manual Review

Recommendation

There should be an improved mechanism for granting roles.

ccashwell commented 6 months ago

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.