Skip to content

Mitigation Confirmed for M-02 #20

Description

@code423n4

Mitigation of M-02: Fully Alleviated

The sponsor adjusted the code to validate the result of the EnumerableSet::add call, yielding a PAYEE_ALREADY_EXISTS error if it evaluates to false. As the OpenZeppelin implementation of EnumerableSet::add will yield a bool indicating whether the entry was newly added to the set, we consider this alleviation adequate for the vulnerability described.

A snippet of the contract with the remediated code showcased can be found below:

/**
 * @notice Updates the payee configuration to a new one.
 * @dev Will release existing fees before the update.
 * @param newPayees Array of  new payees
 * @param newShares Array of shares for each new payee
 */
function changePayees(address[] calldata newPayees, uint256[] calldata newShares) external override onlyManager {
    if (newPayees.length != newShares.length) {
        revert Errors.PAYEES_AND_SHARES_MISMATCHED(newPayees.length, newShares.length);
    }
    if (newPayees.length == 0) {
        revert Errors.NO_PAYEES();
    }
    _releaseIfAvailableIncome();
    uint256 payeesLength = _payees.length();
    if (payeesLength > 0) {
        for (uint256 i = payeesLength; i > 0; i--) {
            address payee = _payees.at(i - 1);
            _payees.remove(payee);
            delete _shares[payee];
            emit PayeeRemoved(payee);
        }
        _totalShares = 0;
    }
    for (uint256 i; i < newPayees.length; i++) {
        if (newPayees[i] == address(0)) {
            revert Errors.CANNOT_SET_TO_ADDRESS_ZERO();
        }
        if (newShares[i] == 0) {
            revert Errors.SHARE_CANNOT_BE_ZERO();
        }

        address payee = newPayees[i];


       /**
        * MITIGATION BLOCK OF M-02 START
        */

        if (!_payees.add(payee)) {
            revert Errors.PAYEE_ALREADY_EXISTS();
        }

        /**
         * MITIGATION BLOCK OF M-02 END
         */

        _shares[payee] = newShares[i];
        _totalShares += newShares[i];

        emit PayeeAdded(payee, newShares[i]);
    }
}

Based on this change, it is no longer possible for the _payees data entry to contain duplicate entries. An accompanying test was introduced to the codebase's KUMAFeeCollector.t.sol file that ensures the correct PAYEE_ALREADY_EXISTS error is yielded whenever a duplicate entry is supplied to the KUMAFeeCollector::changePayees call.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions