Inconsistencies may arise in penalty calculations.

Summary

An attacker can use calling the MinterGateway.sol#_imposePenaltyIfMissedCollateralUpdates function in the MinterGateway.sol#burnM function to create an inconsistency in the penalty calculation imposed on minter.

Vulnerability Detail

In the current protocol, minters impose a penalty on users if they fail to updateCollateral during the Update Collateral Interval period. The MinterGateway.so#updateCollateral function and MinterGateway.sol#burnM function are as follows.

Impact

function updateCollateral(
        uint256 collateral_,
        uint256[] calldata retrievalIds_,
        bytes32 metadataHash_,
        address[] calldata validators_,
        uint256[] calldata timestamps_,
        bytes[] calldata signatures_
    ) external onlyActiveMinter(msg.sender) returns (uint40 minTimestamp_) {
        if (validators_.length != signatures_.length || signatures_.length != timestamps_.length) {
            revert SignatureArrayLengthsMismatch();
        }

        // Verify that enough valid signatures are provided, and get the minimum timestamp across all valid signatures.
        minTimestamp_ = _verifyValidatorSignatures(
            msg.sender,
            collateral_,
            retrievalIds_,
            metadataHash_,
            validators_,
            timestamps_,
            signatures_
        );

        uint240 safeCollateral_ = UIntMath.safe240(collateral_);
        uint240 totalResolvedCollateralRetrieval_ = _resolvePendingRetrievals(msg.sender, retrievalIds_);

        emit CollateralUpdated(
            msg.sender,
            safeCollateral_,
            totalResolvedCollateralRetrieval_,
            metadataHash_,
            minTimestamp_
        );

L202:   _imposePenaltyIfMissedCollateralUpdates(msg.sender);

        _updateCollateral(msg.sender, safeCollateral_, minTimestamp_);

        _imposePenaltyIfUndercollateralized(msg.sender);

        // NOTE: Above functionality already has access to `currentIndex()`, and since the completion of the collateral
        //       update can result in a new rate, we should update the index here to lock in that rate.
        updateIndex();
    }

    function burnM(
        address minter_,
        uint256 maxPrincipalAmount_,
        uint256 maxAmount_
    ) public returns (uint112 principalAmount_, uint240 amount_) {
        if (maxPrincipalAmount_ == 0 || maxAmount_ == 0) revert ZeroBurnAmount();

        bool isActive_ = _minterStates[minter_].isActive;

        if (isActive_) {
            // NOTE: Penalize only for missed collateral updates, not for undercollateralization.
            // Undercollateralization within one update interval is forgiven.
L343:       _imposePenaltyIfMissedCollateralUpdates(minter_);

            (principalAmount_, amount_) = _repayForActiveMinter(
                minter_,
                UIntMath.safe112(maxPrincipalAmount_),
                UIntMath.safe240(maxAmount_)
            );

            emit BurnExecuted(minter_, principalAmount_, amount_, msg.sender);
        } else {
            amount_ = _repayForInactiveMinter(minter_, UIntMath.safe240(maxAmount_));

            emit BurnExecuted(minter_, amount_, msg.sender);
        }

        IMToken(mToken).burn(msg.sender, amount_); // Burn actual M tokens

        // NOTE: Above functionality already has access to `currentIndex()`, and since the completion of the burn
        //       can result in a new rate, we should update the index here to lock in that rate.
        updateIndex();
    }

As shown in L202 and L343, the MinterGateway#_imposePenaltyIfMissedCollateralUpdates function, which imposes a penalty on minters, is called from the MinterGateway#updateCollateral and MinterGateway#burnM functions.

The MinterGateway.sol#_imposePenaltyIfMissedCollateralUpdates function is as follows.

    function _imposePenaltyIfMissedCollateralUpdates(address minter_) internal {
        uint32 updateCollateralInterval_ = updateCollateralInterval();

        MinterState storage minterState_ = _minterStates[minter_];

L744:   (uint40 missedIntervals_, uint40 missedUntil_) = _getMissedCollateralUpdateParameters(
            minterState_.updateTimestamp,
            minterState_.penalizedUntilTimestamp,
            updateCollateralInterval_
        );

        if (missedIntervals_ == 0) return;

        // Save until when the minter has been penalized for missed intervals to prevent double penalizing them.
        _minterStates[minter_].penalizedUntilTimestamp = missedUntil_;

        uint112 principalOfActiveOwedM_ = principalOfActiveOwedMOf(minter_);

        if (principalOfActiveOwedM_ == 0) return;

L759:   _imposePenalty(minter_, uint152(principalOfActiveOwedM_) * missedIntervals_);
}

L744 calculates how many times the user failed to proceed with updateCollateral. Then, calculate the penalty in L759 and impose it on Minter.

The MinterGateway.sol#_imposePenalty function is as follows.

    function _imposePenalty(address minter_, uint152 principalOfPenaltyBase_) internal {
        if (principalOfPenaltyBase_ == 0) return;

        uint32 penaltyRate_ = penaltyRate();

        if (penaltyRate_ == 0) return;

        unchecked {
L715:       uint256 penaltyPrincipal_ = (uint256(principalOfPenaltyBase_) * penaltyRate_) / ONE;

            // As an edge case precaution, cap the penalty principal such that the resulting principal of total active
            // owed M plus the penalty principal is not greater than the max uint112.
            uint256 newPrincipalOfTotalActiveOwedM_ = principalOfTotalActiveOwedM + penaltyPrincipal_;

            if (newPrincipalOfTotalActiveOwedM_ > type(uint112).max) {
                penaltyPrincipal_ = type(uint112).max - principalOfTotalActiveOwedM;
                newPrincipalOfTotalActiveOwedM_ = type(uint112).max;
            }

            // Calculate and add penalty principal to total minter's principal of active owed M
            principalOfTotalActiveOwedM = uint112(newPrincipalOfTotalActiveOwedM_);

            _rawOwedM[minter_] += uint112(penaltyPrincipal_); // Treat rawOwedM as principal since minter is active.

            emit PenaltyImposed(minter_, uint112(penaltyPrincipal_), _getPresentAmount(uint112(penaltyPrincipal_)));
        }
    }

As shown in L715, the penalty is calculated as follows.

//principalOfActiveOwedM_ : principalOfActiveOwedMOf(minter_)
//missedIntervals_ = _getMissedCollateralUpdateParameters()
penaltyPrincipal_ = (uint256(principalOfPenaltyBase_) * penaltyRate) / ONE

In other words, Penalty is calculated linearly in proportion to the number of times users fail to update and the amount of the user's rawOwedM. An attacker can destroy this linear calculation formula by calling the burnM function with very small `maxPriceAmountandmaxAmount_. Let’s look at the attack process in detail. Let's sayupdateCollateralInterval = 24 hour,penaltyRate = 10%, and_rawOwdM[minter1] = 10_000. At this time, let's say thatminter1performsupdateCollateral50 hours after the last update time for various reasons. Then, the penalty imposed on the user should be10_000 2 0.1 = 2_000. However, let's say that the attacker first calls theMinterGateway.sol#burnMfunction forminter125 hours after the last update time and the user proceeds withupdateCollateral. When the attacker calls theMinterGateway.sol#burnMfunction at 25 hours, the penalty is10_000 1 0.1 = 1_000. In this case,_rawOwedM[minter1] = 11_000. Next, ifminterperformsupdateCollateralin 50 hours, the penalty is11_000 1 0.1 = 2_100. It is2_100 > 2_000. In other words, morepenalty` is received by the attacker.

As shown in the picture, an attacker can create an inconsistency in the penalty calculation formula of minter1 at a very small cost. Minters become dissatisfied with the protocol because they impose more penalties than expected.

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L168C1-L211C6 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L331C1-L363C6 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L739C1-L760C6 https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L707C4-L733C6

Tool used

Manual Review

Recommendation

I think the formula for calculating penalty needs to be changed. The penalty calculation formula should not be linear. For example, if it is implemented exponentially, a phenomenon like Woo can be prevented.

Duplicate of #37

sherlock-audit / 2023-10-mzero-judging

FastTiger - Inconsistencies may arise in penalty calculations. #76