Incorrect access control or stale Natspec/docs on `UbiquityDollarToken.setIncentiveContract()`

[0xRizwan]

Title

Incorrect access control or stale Natspec/docs on UbiquityDollarToken.setIncentiveContract()

Vulnerability details

UbiquityDollarToken.setIncentiveContract() is used to set incentive contracts for accounts.

    function setIncentiveContract(address account, address incentive) external {
        require(
            accessControl.hasRole(GOVERNANCE_TOKEN_MANAGER_ROLE, _msgSender()),
            "Dollar: must have admin role"
        );

        incentiveContract[account] = incentive;
        emit IncentiveContractUpdate(account, incentive);
    }

The issue here is with access control given to the above function. The revert message states "Dollar: must have admin role", it means the function is expected to have Admin role as only address who can access the UbiquityDollarToken.setIncentiveContract() function, However, the GOVERNANCE_TOKEN_MANAGER_ROLE is set which seems to be incorrect. On further study on this issue, the readme docs of incentive further confirms that the acces control should be given to Admin role.

Per interface.IIncentive.md,

Dollar admin can set an incentive contract . . .

Per IIncentive.sol

Dollar admin can set an incentive contract . . .

It can be said that the issue either relies with stale or incorrect readme documentation or Natspec or Incorrect function access implementation.

Recommendation

If the UbiquityDollarToken.setIncentiveContract() function is expected to be called by admin then do the following change,

    function setIncentiveContract(address account, address incentive) external {
-        require(
-            accessControl.hasRole(GOVERNANCE_TOKEN_MANAGER_ROLE, _msgSender()),
-            "Dollar: must have admin role"
-        );
+        require(
+            accessControl.hasRole(DEFAULT_ADMIN_ROLE, _msgSender()),
+            "Dollar: must have admin role"
+        );

        incentiveContract[account] = incentive;
        emit IncentiveContractUpdate(account, incentive);
    }

OR . . .

If function implementation is correct then correct the stale docs/Natspec mentioning tokenManager instead of Admin to avoid confusion to code readers.

cc- @pavlovcik @molecula451

[molecula451]

This was purged as a part of the fix-review branch https://github.com/ubiquity/ubiquity-dollar/commit/bb6543eb61e8f328a9b12d20735d4785e9570c2b

[ubiquibot[bot]]

# Issue was not closed as completed. Skipping.