code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/KUMAFeeCollector.sol#L205 https://github.com/code-423n4/2023-02-kuma/blob/3f3d2269fcb3437a9f00ffdd67b5029487435b95/src/kuma-protocol/KUMAFeeCollector.sol#L219

Vulnerability details

Impact

KUMAFeeCollector may emit an event FeeReleased(availableIncome) with availableIncome > 0 when it doesn't actually release any fee.

Proof of Concept

According to the _release() function:

function _release(IERC20 KIBToken, uint256 availableIncome) private {
    uint256 totalShares = _totalShares;

    for (uint256 i; i < _payees.length(); i++) {
        address payee = _payees.at(i);
        KIBToken.safeTransfer(payee, availableIncome * _shares[payee] / totalShares);
    }

    emit FeeReleased(availableIncome);
}

If the length of _payees is 0, it will transfer 0 token, but the event FeeReleased will always be emitted.

The KUMAFeeCollector._release() is called by either KUMAFeeCollector.release() or KUMAFeeCollector._releaseIfAvailableIncome().

The wrong event will never be emitted when calling release() because it will check the length of _payees before calling _release():

function release() external override {
    ...
    if (_payees.length() == 0) {
        revert Errors.NO_PAYEES();
    }
    _release(KIBToken, availableIncome);
}

But, KUMAFeeCollector._releaseIfAvailableIncome() doesn't check the _payees before calling _release(). So, the wrong FeeReleased event may be emitted through _releaseIfAvailableIncome().