xiaoming90 - Minter can retrieve collateral even if its account is underwater

[sherlock-admin4]

xiaoming90

high

Minter can retrieve collateral even if its account is underwater

Summary

Minter can retrieve collateral even if its account is underwater. As a result, the protocol could incur excessive bad debt, the M tokens will not be backed by sufficient collateral assets, there will be an increased risk of insolvency, and the M token might depeg.

Vulnerability Detail

When the minters want to withdraw their collateral, they will execute the proposeRetrieval function with the amount of collateral to be withdrawn. Toward the end of the function (in Line 234), it will call the _revertIfUndercollateralized function to ensure that the withdrawal will not result in the account being undercollateralized.

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L214

File: MinterGateway.sol
214:     function proposeRetrieval(uint256 collateral_) external onlyActiveMinter(msg.sender) returns (uint48 retrievalId_) {
215:         if (collateral_ == 0) revert ZeroRetrievalAmount();
216: 
217:         unchecked {
218:             retrievalId_ = ++_retrievalNonce;
219:         }
220: 
221:         MinterState storage minterState_ = _minterStates[msg.sender];
222:         uint240 currentCollateral_ = minterState_.collateral;
223:         uint240 safeCollateral_ = UIntMath.safe240(collateral_);
224:         uint240 updatedTotalPendingRetrievals_ = minterState_.totalPendingRetrievals + safeCollateral_;
225: 
226:         // NOTE: Revert if collateral is less than sum of all pending retrievals even if there is no owed M by minter.
227:         if (currentCollateral_ < updatedTotalPendingRetrievals_) {
228:             revert RetrievalsExceedCollateral(updatedTotalPendingRetrievals_, currentCollateral_);
229:         }
230: 
231:         minterState_.totalPendingRetrievals = updatedTotalPendingRetrievals_;
232:         _pendingCollateralRetrievals[msg.sender][retrievalId_] = safeCollateral_;
233: 
234:         _revertIfUndercollateralized(msg.sender, 0);
235: 
236:         emit RetrievalCreated(retrievalId_, msg.sender, safeCollateral_);
237:     }

However, the _revertIfUndercollateralized function does not consider the account's pending penalty to the owned principal due to missed collateral updates. Thus, the owned M used in the check is undervalued, allowing the account to withdraw its collateral even if the account is already under-collateralized when factoring in the pending penalties incurred.

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L1018

File: MinterGateway.sol
1018:     function _revertIfUndercollateralized(address minter_, uint240 additionalOwedM_) internal view {
1019:         uint256 maxAllowedActiveOwedM_ = maxAllowedActiveOwedMOf(minter_);
1020: 
1021:         // If the minter's max allowed active owed M is greater than the max uint240, then it's definitely greater than
1022:         // the max possible active owed M for the minter, which is capped at the max uint240.
1023:         if (maxAllowedActiveOwedM_ >= type(uint240).max) return;
1024: 
1025:         unchecked {
1026:             uint256 finalActiveOwedM_ = uint256(activeOwedMOf(minter_)) + additionalOwedM_;
1027: 
1028:             if (finalActiveOwedM_ > maxAllowedActiveOwedM_) {
1029:                 revert Undercollateralized(finalActiveOwedM_, maxAllowedActiveOwedM_);
1030:             }
1031:         }
1032:     }

In addition, the activeOwedMOf function used in Line 1026 above in the _revertIfUndercollateralized function also does not take into consideration the account's pending penalties.

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L513

File: MinterGateway.sol
513:     function activeOwedMOf(address minter_) public view returns (uint240) {
514:         // NOTE: This should also include the present value of unavoidable penalities. But then it would be very, if
515:         //       not impossible, to determine the `totalActiveOwedM` to the same standards.
516:         return
517:             _minterStates[minter_].isActive
518:                 ? _getPresentAmount(uint112(_rawOwedM[minter_])) // Treat rawOwedM as principal since minter is active.
519:                 : 0;
520:     }

The following simplified example illustrates the issue. For simplicity's sake, assume the minter rate is zero, and the mint ratio is 100%.

1. At T1, Bob's maxAllowedActiveOwedM_ is 100 based on his collateral assets, and he minted 50 M tokens. Thus, his activeOwedMOf will be 50 M tokens.

2. At T50, Bob did not call the collateral updates for many epochs. As a result, he accumulated a penalty of 100 M tokens for all his missed updates. If one calls the getPenaltyForMissedCollateralUpdates against Bob's account, it will return 100 M tokens as Bob's penalty. If someone intends to repay Bob's debt now, they must repay a total debt of 150 M tokens (50 + 100). Bob's account is technically underwater at this point (Debt > maxAllowedActiveOwedM_).

3. At T51, Bob decided to retrieve 45 of his collateral. When the _revertIfUndercollateralized function is executed to check whether Bob's account is undercollateralized via the following logic and determine that Bob's account is still healthy:

   maxAllowedActiveOwedM_ = 100 - 45 (withdraw) = 55
   activeOwedMOf(Bob) = 50
   
   finalActiveOwedM_ = activeOwedMOf(Bob) + additionalOwedM_ = 50 + 0 = 50
   
   finalActiveOwedM_ > maxAllowedActiveOwedM_
   50 > 55 => False => No Revert

4. The protocol will proceed to facilitate the retrieval of 45 collateral for Bob since the protocol deems his account healthy, and his collateral will decrease to 55.

5. Before the retrieval, 150 debts were backed by 100 collateral. After the retrieval, the 150 debts were backed by 55 collateral. The account and protocol are in a worse position after the retrieval, and its M tokens are backed by fewer collateral. This indicates that the collateral checks and the mechanism to ensure the protocol's solvency are inadequate.

Impact

The protocol could incur excessive bad debt, the M tokens will not be backed by sufficient collateral assets, there will be an increased risk of insolvency, and the M token might depeg.

Code Snippet

https://github.com/sherlock-audit/2023-10-mzero/blob/main/protocol/src/MinterGateway.sol#L214

Tool used

Manual Review

Recommendation

When determining if an account is undercollateralized during retrieval, the protocol should consider the account's debt in its entirety, including the debt from the accumulated penalties. This will prevent minters whose accounts are already in an unhealthy position due to incurred penalties from retrieving their collateral, which causes the owned M tokens to be backed by even fewer collateral after the transaction.

Duplicate of #63

[toninorair]

Issue is a duplicate of 63 and is invalid for the same reason

If Bob did not call the collateral updates for many epochs _revertIfUndercollateralized won't proceed. maxAllowedActiveOwedMOf will be 0 because collateralOf will be 0 because block.timestamp >= collateralExpiryTimestampOf(minter_)