sammy - EDITION_MINTER_ROLE is not configurable as grantRoles() cannot be called in Edition.sol

[sherlock-admin4]

sammy

medium

EDITION_MINTER_ROLE is not configurable as grantRoles() cannot be called in Edition.sol

Summary

Quoting the QnA :

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.

However, this role cannot be assigned to any address.

Vulnerability Detail

The grantRoles() function in Edition.sol can only be called by the EDITION_MANAGER_ROLE role :

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

This role is assigned to the TitlesCore.sol contract when an Edition.sol contract is initialized. However, there is no function in TitlesCore.sol that calls the grantRoles() function of Edition.sol, making it impossible to invoke it to configure EDITION_MINTER_ROLE.

Impact

Only the owner of the Edition.sol contract can mint promo tokens.

Code Snippet

Tool used

Manual Review

Recommendation

Make the following change to grantRoles() :

-     onlyRoles(EDITION_MANAGER_ROLE)
+    onlyOwnerOrRoles(EDITION_MANAGER_ROLE)

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.

[pqseags]

Note that this is incorrectly tagged as a duplicate of #266, which is unrelated

[Hash01011122]

Thanks for your input invalidating this one

[sammy-tm]

Escalate

Contest readme states the following two points :

Are there any protocol roles? Please list them and provide whether they are TRUSTED or RESTRICTED, or provide a more comprehensive description of what a role can and can't do/impact.

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.

Should potential issues, like broken assumptions about function behavior, be reported if they could pose risks in future integrations, even if they might not be an issue in the context of the scope? If yes, can you elaborate on properties/invariants that should hold?

Yes.

The information about the code being "upgraded" in the future to support the functionality of the role was not available during the contest. According to the README, the role should be able to call the promoMint function. However, this is not possible as the role itself isn't configurable. This breaks a key invariant/disagrees with the intended protocol functionality(acc to the README). Therefore, I see this issue as valid with medium severity.

[sherlock-admin3]

Escalate

Contest readme states the following two points :

Are there any protocol roles? Please list them and provide whether they are TRUSTED or RESTRICTED, or provide a more comprehensive description of what a role can and can't do/impact.

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.

Should potential issues, like broken assumptions about function behavior, be reported if they could pose risks in future integrations, even if they might not be an issue in the context of the scope? If yes, can you elaborate on properties/invariants that should hold?

Yes.

The information about the code being "upgraded" in the future to support the functionality of the role was not available during the contest. According to the README, the role should be able to call the promoMint function. However, this is not possible as the role itself isn't configurable. This breaks a key invariant/disagrees with the intended protocol functionality(acc to the README). Therefore, I see this issue as valid with medium severity.

You've created a valid escalation!

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.

[realfugazzi]

I believe this is a valid issue, documented it myself in #166

[0xjuaan]

I also believe this is valid, reported in #240

[cvetanovv]

I agree with the escalation. This issue is a duplicate of #166. The outcome there will decide whether it is valid.

[WangSecurity]

Indeed, the watsons are correct here, planning to accept the escalation and create a new family of medium severity with the core issue of "roles are set incorrectly or not set at all" and duplicate with #148 (also in #148 you can see a more comprehensive list of duplicates).

[KupiaSecAdmin]

This is a dup of #400

[Evert0x]

Result:
Medium
Duplicate of #148

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• sammy-tm: accepted