code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L140-L148 https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L197-L198

Vulnerability details

Summary

The setGovernanceChainIdentifier() is used to specify whether the current chain is thegovernance chain and allows the TAP emission to happen on this chain. There is however no protection against multiple chains being set as the governance chain which would lead to an increased unbridled emission rate. Moreover the dso_supply is not tracked accross multiple chains. This means even in the normal use case of switching from one chain to the other the emission rate (and total supply cap) would be reset to the initial value which again creates an increased emission rate and total supply cap.

Vulnerability Detail

The setGovernanceChainIdentifier() in the TapOFT contract determines which chain is the governance:

    function setGovernanceChainIdentifier(
        uint256 _identifier
    ) external onlyOwner {
        emit GovernanceChainIdentifierUpdated(
            governanceChainIdentifier,
            _identifier
        );
        governanceChainIdentifier = _identifier;
    }

The allowed emission per week are dependent on this setting:

function emitForWeek() external notPaused returns (uint256) {
    require(_getChainId() == governanceChainIdentifier, "chain not valid");
    ...
}

There is however no protection against multiple chains being set as the governance chain simultaneously (even temporarily). This makes it possible for the emission to take place on multiple chains simultaneously when 2 or more TapOFT contracts have been configured to be on the governance chain at the same time resulting in an increased emission rate.

Also in the normal case when the governance chain would be 'moved' from one chain to the other the dso_supply is not tracked across the chains, meaning the dso_supply is reset to the initial value when switched to the new governance chain. This again creates an increased emission rate and ultimately total supply cap.

Impact

Although this is dependent on a misconfiguration (or owner key/multisig compromise it is highly surprising given that this feature is not handled cross chain given that the protocol is built around LayerZero's cross chain messaging for other purposes.

Tool used

Manual Review

Recommendation

Use LayerZero's cross chain messaging to make sure the dso_supply is tracked across multiple chains and only one chain can be set as the governance chain. Alternatively make the governance chain an immutable property of the system, making sure that even when a new chain is added this chain cannot be added to the system with the chain set as governance chain.

Assessed type

Governance

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Owner misconfiguration. Consider QA

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)