<!-- canonical: https://0xsimao.com/findings/titles-publishing-protocol-fee-cannot-grant-revoke -->

# FeeManager's admin cannot grant or revoke any role

Medium · Sherlock · Referential NFT publishing · 22nd April 2024

Finding M-4 of the TITLES Publishing Protocol competition.

- Protocol: https://audits.sherlock.xyz/contests/326
- Codebase: https://github.com/0xsimao/2024-04-titles/tree/d7f60952df22da00b772db5d3a8272a988546089
- Source: https://github.com/sherlock-audit/2024-04-titles-judging/issues/148

---

## 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);
}
```

---

Related findings:

- [Admin can unrestrictedly affect the odds of a raffle by setting themselves up with role(1) in `WinnablesTicket`](https://0xsimao.com/findings/winnables-raffles-unrestrictedly-affect-odds-themselves): Winnables Raffles
- [Attacker will prevent any raffles by calling `WinnablesTicketManager::cancelRaffle` before admin starts raffle](https://0xsimao.com/findings/winnables-raffles-raffles-winnables-raffle-starts): Winnables Raffles
- [Fixed term loans can be deployed with a wrong fee manager and possibly steal all funds.](https://0xsimao.com/findings/maple-finance-iii-loans-deployed-fee-steal): Maple Finance