Skip to content
Request an audit

‹ All findings

FeeManager's admin cannot grant or revoke any role

MediumTITLES Publishing Protocol·Sherlock · Referential NFT publishing · 22nd April 2024CodebaseM-4

Source

Summary

The FeeManager contract lacks an interface for the admin to grant or revoke roles.

Vulnerability Detail

The contest's README states:

bash
ADMIN_ROLE
...
On FeeManager, this role can:
...
3) Grant or revoke any role to/from any address (`grantRole`, `revokeRole`).

The FeeManager contract, inherited from solady's OwnableRoles contract, only permits the owner to manage roles.

solidity
/// @dev Allows the owner to grant `user` `roles`.
/// If the `user` already has a role, then it will be a no-op for the role.
function grantRoles(address user, uint256 roles) public payable virtual onlyOwner {
    _grantRoles(user, roles);
}

/// @dev Allows the owner to remove `user` `roles`.
/// If the `user` does not have a role, then it will be a no-op for the role.
function revokeRoles(address user, uint256 roles) public payable virtual onlyOwner {
    _removeRoles(user, roles);
}

The FeeManager contract itself does not provide any interfaces that enable the admin to grant or revoke roles.

Impact

The admin cannot grant or revoke any roles. This limitation cannot be changed since the contract is not upgradable. A new role can only be granted or revoked by the owner, which is the TitlesCore contract, and it also lacks the corresponding functions.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/README.md#q-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-cant-doimpact https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L115

Tool used

Manual Review

Recommendation

Implement admin interfaces to manage roles

solidity
function grantRoles(address guy, uint256 roles)
    public
    payable
    override
    onlyOwnerOrRoles(ADMIN_ROLE)
{
    _grantRoles(guy, roles);
}

function revokeRoles(address guy, uint256 roles)
    public
    payable
    override
    onlyOwnerOrRoles(ADMIN_ROLE)
{
    _removeRoles(guy, roles);
}