Issue in access controls leads to not being able to create pool at all or pool loses it's mint and burning functionality

Summary

Access control does not have all the access control library functions exposed which leads to not being able to grant the minter role to ubiquity pool and whole functionality bricks.

Vulnerability Detail

First let's look at how the access controls are initialized in the deployment of diamond, which can be found in diamondInIt.sol which is out of scope but directly impacts all the implementations that are in scope.

DiamondInit.sol

    function init(Args memory _args) external {
        // adding ERC165 data
        LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();
        ds.supportedInterfaces[type(IERC165).interfaceId] = true;
        ds.supportedInterfaces[type(IDiamondCut).interfaceId] = true;
        ds.supportedInterfaces[type(IDiamondLoupe).interfaceId] = true;
        ds.supportedInterfaces[type(IERC173).interfaceId] = true;

        LibAccessControl.grantRole(DEFAULT_ADMIN_ROLE, _args.admin);
        LibAccessControl.grantRole(GOVERNANCE_TOKEN_MINTER_ROLE, _args.admin);
        LibAccessControl.grantRole(GOVERNANCE_TOKEN_BURNER_ROLE, _args.admin);
        LibAccessControl.grantRole(CREDIT_TOKEN_MINTER_ROLE, _args.admin);
        LibAccessControl.grantRole(CREDIT_TOKEN_BURNER_ROLE, _args.admin);
        LibAccessControl.grantRole(DOLLAR_TOKEN_MINTER_ROLE, _args.admin);
        LibAccessControl.grantRole(DOLLAR_TOKEN_BURNER_ROLE, _args.admin);
        LibAccessControl.grantRole(PAUSER_ROLE, _args.admin);
        LibAccessControl.grantRole(CREDIT_NFT_MANAGER_ROLE, _args.admin);
        LibAccessControl.grantRole(STAKING_MANAGER_ROLE, _args.admin);
        LibAccessControl.grantRole(INCENTIVE_MANAGER_ROLE, _args.admin);
        LibAccessControl.grantRole(GOVERNANCE_TOKEN_MANAGER_ROLE, _args.admin);

So we can see all the roles are assigned to admin which is an EOA, and no roles are granted to pool.

One may argue that later using the access control facet, the roles can be granted to pool, but let's look at the code there :

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/AccessControlFacet.sol#L20-L26

    function grantRole(
        bytes32 role,
        address account
    ) external onlyRole(_getRoleAdmin(role)) {
        return _grantRole(role, account);
    }

So the function has onlyRole modifier, which checks for an admin role, now the problem is this is uncallable for the reason that in DiamondInIt the admin was never set, which means the pool can't mint and burn token and its functionality is completely bricked.

if in DiamondInit.sol the the admin role is granted to the pool:

It bricks all other roles,
Secondly still we would need to give a minter role to EOA because without a minter role dollar cannot be minted to create a dollar/3crv pool and unless there is that pool, the ubiquity pool can't fetch a price, so it becomes a looping fallacy in that case.

Another problem is:

Access control also doesn't allow to revoke of any roles too, which means if someone turns malicious his/her role cannot be revoked at all because the revoking function has the same modifier and is uncallable because the admin was not set in diamondinit.sol

And also setRoleAdmin function from LibAccessControl.sol function is not exposed in facet and is also never used to set admin for any roles in deployment script at:

https://github.com/ubiquity/ubiquity-dollar/blob/development/packages/contracts/migrations/development/Deploy001_Diamond_Dollar.s.sol

Impact

UbiquityPool.sol is bricked, minting and redeeming in the current state is impossible.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/AccessControlFacet.sol#L20-L26 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L1-L840

Tool used

Brain Cats

Recommendation

Grant an admin role to all roles in DiamondInit.sol

Duplicate of #90

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

DEFAULT_ADMIN_ROLE is already granted