code423n4 commented 1 year ago

Lines of code

Vulnerability details

Impact

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

Proof of Concept

Instances (47):

File: src/kuma-protocol/KBCToken.sol

101:         if (!IAccessControl(_KUMAAddressProvider.getAccessController()).hasRole(Roles.KUMA_MANAGER_ROLE, msg.sender)) {

File: src/kuma-protocol/KIBToken.sol

42:     modifier onlyRole(bytes32 role) {

100:     function setEpochLength(uint256 epochLength) external override onlyRole(Roles.KUMA_SET_EPOCH_LENGTH_ROLE) {

131:         onlyRole(Roles.KUMA_MINT_ROLE.toGranularRole(_riskCategory))

162:         onlyRole(Roles.KUMA_BURN_ROLE.toGranularRole(_riskCategory))

371:     function _authorizeUpgrade(address newImplementation) internal view override onlyRole(Roles.KUMA_MANAGER_ROLE) {}

File: src/kuma-protocol/KUMAAccessController.sol

7: contract KUMAAccessController is AccessControl {

File: src/kuma-protocol/KUMAAddressProvider.sol

15:     IAccessControl private _accessController;

41:     function initialize(IAccessControl accessController) external override initializer {

File: src/kuma-protocol/KUMASwap.sol

57:     modifier onlyRole(bytes32 role) {

58:         if (!IAccessControl(_KUMAAddressProvider.getAccessController()).hasRole(role, msg.sender)) {

245:         onlyRole(Roles.KUMA_MANAGER_ROLE)

274:         onlyRole(Roles.KUMA_SWAP_CLAIM_ROLE.toGranularRole(_riskCategory))

342:     function pause() external override onlyRole(Roles.KUMA_SWAP_PAUSE_ROLE.toGranularRole(_riskCategory)) {

349:     function unpause() external override onlyRole(Roles.KUMA_SWAP_UNPAUSE_ROLE.toGranularRole(_riskCategory)) {

359:     function setFees(uint16 variableFee, uint256 fixedFee) external override onlyRole(Roles.KUMA_MANAGER_ROLE) {

372:         onlyRole(Roles.KUMA_MANAGER_ROLE)

385:     function initializeDeprecationMode() external override onlyRole(Roles.KUMA_MANAGER_ROLE) whenNotDeprecated {

398:     function uninitializeDeprecationMode() external onlyRole(Roles.KUMA_MANAGER_ROLE) whenNotDeprecated {

412:     function enableDeprecationMode() external override onlyRole(Roles.KUMA_MANAGER_ROLE) whenNotDeprecated {

570:     function _authorizeUpgrade(address newImplementation) internal view override onlyRole(Roles.KUMA_MANAGER_ROLE) {}

File: src/kuma-protocol/MCAGRateFeed.sol

17:     IAccessControl private _accessController;

33:     function initialize(IAccessControl accessController) external override initializer {

File: src/kuma-protocol/interfaces/IKUMAAddressProvider.sol

17:     function initialize(IAccessControl accessController) external;

File: src/kuma-protocol/interfaces/IMCAGRateFeed.sol

11:     function initialize(IAccessControl accessController) external;

File: src/mcag-contracts/AccessController.sol

7: contract AccessController is AccessControl {

File: src/mcag-contracts/Blacklist.sol

10:     IAccessControl public immutable override accessController;

24:     constructor(IAccessControl _accessController) {

File: src/mcag-contracts/KUMABondToken.sol

18:     IAccessControl public immutable override accessController;

26:     modifier onlyRole(bytes32 role) {

44:     constructor(IAccessControl _accessController, IBlacklist _blacklist) ERC721("KUMA Bonds", "KUMA") {

67:         onlyRole(Roles.MCAG_MINT_ROLE)

86:     function redeem(uint256 tokenId) external override onlyRole(Roles.MCAG_BURN_ROLE) whenNotPaused {

100:     function setUri(string memory newUri) external override onlyRole(Roles.MCAG_SET_URI_ROLE) {

108:     function pause() external override onlyRole(Roles.MCAG_PAUSE_ROLE) {

115:     function unpause() external override onlyRole(Roles.MCAG_UNPAUSE_ROLE) {

File: src/mcag-contracts/KYCToken.sol

16:     IAccessControl public immutable accessController;

24:     modifier onlyRole(bytes32 role) {

34:     constructor(IAccessControl _accessController) ERC721("MCAG KYC Token", "MKYCT") {

49:     function mint(address to, KYCData calldata kycData) external override onlyRole(Roles.MCAG_MINT_ROLE) {

63:     function burn(uint256 tokenId) external override onlyRole(Roles.MCAG_BURN_ROLE) {

79:     function setUri(string memory newUri) external override onlyRole(Roles.MCAG_SET_URI_ROLE) {

File: src/mcag-contracts/MCAGAggregator.sol

13:     IAccessControl public immutable accessController;

21:     modifier onlyRole(bytes32 role) {

33:     constructor(string memory description_, int256 maxAnswer_, IAccessControl _accessController) {

52:     function transmit(int256 answer) external override onlyRole(Roles.MCAG_TRANSMITTER_ROLE) {

69:     function setMaxAnswer(int256 newMaxAnswer) external onlyRole(Roles.MCAG_MANAGER_ROLE) {

Tools Used

https://github.com/Picodes/4naly3er

GalloDaSballo commented 1 year ago

Due to a miscomunication 4nalyz3r wasn't run, hence Admin Privilege Reports should be in-scope per the rules.

Per the rules I also have to ask for a certain degree of proof and submission quality in order to award higher severity

Due to this, am electing to downgrade to QA, for lack of specific attacks being demonstrated

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

m19 marked the issue as sponsor disputed

m19 commented 1 year ago

We dispute the validity of this: the mcag-contracts are per definition centralized since they present real-world assets and thus require a centralized entity to issue those.

For the kuma-protocol contracts all the relevant roles will be granted to the KUMA DAO and the protocol cannot function if there are no ways to adjust the configuration. I don't see any way for the KUMA DAO to "drain funds" either even if the DAO was malicious.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-a

code-423n4 / 2023-02-kuma-findings

Centralization Risk for trusted owners #9

Lines of code

Vulnerability details

Impact

Proof of Concept

Tools Used