code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

Functionalities for burning bHermesVotes, bHermesGauges, and bHermesBoost tokens are unavailable even though related functions, which are inaccessible externally, for burning these tokens do exist to indicate needs for such functionalities #897

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/tokens/bHermesVotes.sol#L35-L37 https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/tokens/bHermesVotes.sol#L39-L42 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Gauges.sol#L485-L488 https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L302-L304

Vulnerability details

Impact

The bHermesVotes contract has the following bHermesVotes.burn function, which is only callable by the bHermes contract due to the bHermesVotes.onlybHermes modifier that is called by this function. However, the bHermes contract does not have a function that calls the bHermesVotes.burn function. Hence, although the function for burning the bHermesVotes tokens exists, there is no way to burn such tokens should the need for burning such tokens, such as for burning these owned by the bHermes contract, arise.

https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/tokens/bHermesVotes.sol#L35-L37

    function burn(address from, uint256 amount) external onlybHermes {
        _burn(from, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/53c7fe9d5e55754960eafe936b6cb592773d614c/src/hermes/tokens/bHermesVotes.sol#L39-L42

    modifier onlybHermes() {
        if (msg.sender != bHermes) revert NotbHermes();
        _;
    }

Similarly, the following ERC20Gauges and ERC20Boost contracts have the ERC20Gauges._burn and ERC20Boost._burn functions, which are both internal, but the bHermesGauges contract that inherits the ERC20Gauges contract and the bHermesBoost contract that inherits the ERC20Boost contract do not have external or public functions that can call these internal ERC20Gauges._burn and ERC20Boost._burn functions. Thus, there are also no ways to burn these bHermesGauges and bHermesBoost tokens should the needs for burning such tokens, such as for burning these owned by the bHermes contract, arise.

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Gauges.sol#L485-L488

    function _burn(address from, uint256 amount) internal virtual override {
        _decrementWeightUntilFree(from, amount);
        super._burn(from, amount);
    }

https://github.com/code-423n4/2023-05-maia/blob/62f4f01a522dcbb4b9abfe2f6783bbb67c0da022/src/erc-20/ERC20Boost.sol#L302-L304

    function _burn(address from, uint256 amount) internal override notAttached(from, amount) {
        super._burn(from, amount);
    }

As a result, the functionalities for burning the bHermesVotes, bHermesGauges, and bHermesBoost tokens are unavailable even though the related functions, which are inaccessible externally, for burning these tokens do exist to indicate the needs for such functionalities.

Proof of Concept

The following steps can occur for the described scenario involving the bHermesVotes contract. The scenarios involving the bHermesGauges and bHermesBoost contracts are similar to this.

  1. 1% of the minted bHermesVotes tokens owned by the bHermes contract need to be burned according to the consensus reached by the protocol's users and admins.
  2. The bHermesVotes.burn function could be called to burn 1% of the minted bHermesVotes tokens owned by the bHermes contract if the bHermes contract can call such function.
  3. Yet, because the bHermes contract does not have a function that calls the bHermesVotes.burn function, the bHermesVotes.burn function cannot be called at all. As a result, none of the minted bHermesVotes tokens can be burned, and the need for burning 1% of the minted bHermesVotes tokens owned by the bHermes contract cannot be fulfilled.

Tools Used

VSCode

Recommended Mitigation Steps

The bHermes contract can be updated to add a function, which is only callable by the trusted admins, that can call the bHermesVotes.burn function. Similarly, functions, which are only callable by the bHermes contract, that can call the ERC20Gauges._burn and ERC20Boost._burn functions can be added in the bHermesGauges and bHermesBoost contracts; then, the bHermes contract can add functions, which are only callable by the trusted admins, that can call these new functions added in the bHermesGauges and bHermesBoost contracts. To be more restrictive, these new functions added in the bHermes contract can be required to be only able to burn the bHermesVotes, bHermesGauges, and bHermesBoost tokens owned by the bHermes contract.

Yet, if the functionalities for burning the bHermesVotes, bHermesGauges, and bHermesBoost tokens are indeed not needed, then the bHermesVotes.burn function needs to be removed, and no new functions should be added in the bHermesGauges and bHermesBoost contracts to ensure that the ERC20Gauges._burn and ERC20Boost._burn functions would not be called.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as primary issue

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-sponsor commented 1 year ago

0xLightt marked the issue as sponsor acknowledged

c4-sponsor commented 1 year ago

0xLightt marked the issue as disagree with severity

0xLightt commented 1 year ago

The reason burning only exists in bHermesVotes is for it to be used in PartnerUtilityManager and ERC4626PartnerManager here.

trust1995 commented 1 year ago

Will demote to QA due to impact considerations.

c4-judge commented 1 year ago

trust1995 changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

trust1995 marked the issue as grade-c