search

sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

iberry - Unrestricted access to the init function in the DiamondInit.sol contract can invalidate all access rights of the contract (LibAccessControl.sol). #118

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

iberry

high

Unrestricted access to the init function in the DiamondInit.sol contract can invalidate all access rights of the contract (LibAccessControl.sol).

Summary

This is only one interface to init all rights, such as grantRole(DEFAULT_ADMIN_ROLE, _args.admin).Unrestricted access to the init function in the DiamondInit.sol contract can invalidate all access rights of the contract.

Vulnerability Detail

The "init" function in the "DiamondInit.sol" contract does not have access control and can be called by any external party to set someone to control all rights.

Impact

high

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/upgradeInitializers/DiamondInit.sol#L46-L105

function init(Args memory _args) external {
    // adding ERC165 data
   ....

    LibAccessControl.grantRole(DEFAULT_ADMIN_ROLE, _args.admin);               // bug  begin.
    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);

   ......
}

}

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, it is advisable to implement proper access controls to restrict the execution of the init function to authorized addresses only. Access control can be enforced using modifiers, roles, or other mechanisms to ensure that only trusted entities can initialize the contract or flag to check one time.

sherlock-admin2 commented 8 months ago

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

auditsea commented:

It's proxy contract, and the init function will not be registered as facet

sherlock-admin2 commented 8 months ago

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

auditsea commented:

It's proxy contract, and the init function will not be registered as facet

nevillehuang commented 8 months ago

Invalid, front-running intializers ares not valid based on sherlock rules

  1. Front-running initializers: Front-running initializers where there is no irreversible damage or loss of funds & the protocol could just redeploy and initialize again is not a valid issue.