sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

nisedo - Broken Role Checking Mechanism #337

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

nisedo

high

Broken Role Checking Mechanism

Summary

The current role checking feature is flawed because it verifies only the presence of a role for an address, without confirming if the address holds the correct role for the specific Edition/Work.

Vulnerability Detail

According to the README, the roles EDITION_MANAGER_ROLE, EDITION_PUBLISHER_ROLE, and EDITION_MINTER_ROLE should be confined to their respective contexts.

The sponsor mentioned in the Discord channel:

Edition owners and Work creators are trusted in the context of their own respective Editions/Works but not in the context of the broader ecosystem including other Editions/Works

However, the existing role checking system merely verifies that an address possesses a role, disregarding the specific Edition/Work to which this role applies. Consequently, an address with the EDITION_MANAGER_ROLE can act on all Editions, and similarly for other roles.

    /// @dev Marks a function as only callable by an account with `roles`.
    modifier onlyRoles(uint256 roles) virtual {
        _checkRoles(roles);
        _;
    }
    /// @dev Throws if the sender does not have any of the `roles`.
    function _checkRoles(uint256 roles) internal view virtual {
        /// @solidity memory-safe-assembly
        assembly {
            // Compute the role slot.
            mstore(0x0c, _ROLE_SLOT_SEED)
            mstore(0x00, caller())
            // Load the stored value, and if the `and` intersection
            // of the value and `roles` is zero, revert.
            if iszero(and(sload(keccak256(0x0c, 0x20)), roles)) {
                mstore(0x00, 0x82b42900) // `Unauthorized()`.
                revert(0x1c, 0x04)
            }
        }
    }

For instance, a malicious user with the EDITION_MANAGER_ROLE or EDITION_MINTER_ROLE could exploit Edition.promoMint() to issue the entire collection of tokens from another Edition not owned by them.

Impact

The flawed role system allows unauthorized users to act on other users' Editions and Works.

Code Snippet

Tool used

Manual Review

Recommendation

Implement role checks using the OpenZeppelin Access Control library to ensure roles are contextually appropriate.