KUMAFeeCollector - duplicate payee can be added

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/main/src/kuma-protocol/KUMAFeeCollector.sol#L152

Vulnerability details

Impact

In KUMAFeeCollector contract, the addPayee validates that an already present payee cannot be added again to the _payees set.

    function addPayee(address payee, uint256 share) external override onlyManager {
        if (_payees.contains(payee)) {
            revert Errors.PAYEE_ALREADY_EXISTS();
        }
        // ...
    }

However a similar check is not present in the changePayees function. https://github.com/code-423n4/2023-02-kuma/blob/main/src/kuma-protocol/KUMAFeeCollector.sol#L152

So a single payee can be added multiple times to the _payees set using the changePayees function.

Proof of Concept

Consider this scenario:

• KUMAFeeCollector is deployed and Account1 and Account2 are intended to be added as payees with 10 share each.
• Using the addPayee function each account can be added only once.
• But using the changePayees function, Account1 can be added twice to the _payees set.

The final payees list looks like this:

• Account1 - 10 shares
• Account2 - 10 shares
• Account1 - 10 shares
• _totalShares = 30

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding a check in changePayees function also which prevent addition of duplicate payee entries.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #13

[GalloDaSballo]

Awarding 50% due to lack of detail in terms what goes wrong vs primary

[c4-judge]

GalloDaSballo marked the issue as partial-50

[akshaysrivastav]

Hey @GalloDaSballo I think this report also mentions all the necessary technical details as menttioned in #13. Just that, I intended the report to be precise and small.

As you can see, the example scenario above shows the exact _totalShares and individual shares values.

[c4-judge]

GalloDaSballo marked the issue as full credit

[GalloDaSballo]

After re-reading, I agree that the finding shows the issue with accounting, restoring full credit

[c4-judge]

GalloDaSballo marked the issue as satisfactory