Morpho Blue report in review

So I per-took in this contest and had zero bugs confirmed, so now below are the notable bugs I understudied on the contest.

Liquidation seizeAssets computation rounding issue #461

Author: cmichel

Morpho Blue, like every other money market allows users position to be liquidated when borrowed asset is above LLTV. Morpho design assigns shares to users as representation of their borrow position in a given market. This share system is same as is available in an EIP4626 style market. Only difference is that a Morpho is a singleton contract, that holds all markets and its operation in the single contract. So it represent shares belonging to each market in a array of structs. Like so:

struct Position {
uint256 supplyShares;
uint128 borrowShares;
uint128 collateral;
}

mapping(Id => mapping(address => Position)) public position;

SupplyShares representing the lenders/suppliers increasing assets in the market supplied to. And borrowShares representing users share of borrowed assets.

struct Market {
uint128 totalSupplyAssets;
uint128 totalSupplyShares;
uint128 totalBorrowAssets;
uint128 totalBorrowShares;
uint128 lastUpdate;
uint128 fee;
}

struct MarketParams {
address loanToken;
address collateralToken;
address oracle;
address irm;
uint256 lltv;
}

Each market is defined by the MarketParams at market creation and Market struct is updated accordingly with regards to the current state iterations on the selected market.

When supplying , borrowing , liquidating, withdrawing or repaying , a user can either select to specify the required amount of assets to action as shares or assets, but never both at same time.

require(UtilsLib.exactlyOneZero(seizedAssets, repaidShares), ErrorsLib.INCONSISTENT_INPUT);

The rounding issue occurs when a liquidator is attempting to convert shares to assets on liquidation.

Ex.

Alice has an unhealthy position in a WETH/USDT market.

Collateral= 4 wei
LLTV= 75%
Collateral price= 1$
BorrowCapacity = (4 * 1) * 0.75 = 3$
Borrowed Asset= 3USDT
borrowShare = 3e6
Supplied Asset = 3USDT
Should the price of WETH drop to 0.80$
Borrow Capacity: (4 * 0.8$) * 0.75 = 2.4$

 function liquidate(
        MarketParams memory marketParams,
        address borrower,
        uint256 seizedAssets,
        uint256 repaidShares,
        bytes calldata data
    ) external returns (uint256, uint256) {
        Id id = marketParams.id();
        require(market[id].lastUpdate != 0, ErrorsLib.MARKET_NOT_CREATED);
        require(UtilsLib.exactlyOneZero(seizedAssets, repaidShares), ErrorsLib.INCONSISTENT_INPUT);

        _accrueInterest(marketParams, id);

        uint256 collateralPrice = IOracle(marketParams.oracle).price();

        require(!_isHealthy(marketParams, id, borrower, collateralPrice), ErrorsLib.HEALTHY_POSITION);

        uint256 repaidAssets;
        {
            // The liquidation incentive factor is min(maxLiquidationIncentiveFactor, 1/(1 - cursor*(1 - lltv))).
            uint256 liquidationIncentiveFactor = UtilsLib.min(
                MAX_LIQUIDATION_INCENTIVE_FACTOR,
                WAD.wDivDown(WAD - LIQUIDATION_CURSOR.wMulDown(WAD - marketParams.lltv))
            );

            if (seizedAssets > 0) {
                repaidAssets =
                    seizedAssets.mulDivUp(collateralPrice, ORACLE_PRICE_SCALE).wDivUp(liquidationIncentiveFactor);
                repaidShares = repaidAssets.toSharesDown(market[id].totalBorrowAssets, market[id].totalBorrowShares);
            } else {
                repaidAssets = repaidShares.toAssetsUp(market[id].totalBorrowAssets, market[id].totalBorrowShares);
                seizedAssets =
                    repaidAssets.wMulDown(liquidationIncentiveFactor).mulDivDown(ORACLE_PRICE_SCALE, collateralPrice);
            }
        }

        position[id][borrower].borrowShares -= repaidShares.toUint128();
        market[id].totalBorrowShares -= repaidShares.toUint128();
        market[id].totalBorrowAssets = UtilsLib.zeroFloorSub(market[id].totalBorrowAssets, repaidAssets).toUint128();

        position[id][borrower].collateral -= seizedAssets.toUint128();

        uint256 badDebtShares;
        if (position[id][borrower].collateral == 0) {
            badDebtShares = position[id][borrower].borrowShares;
            uint256 badDebt = UtilsLib.min(
                market[id].totalBorrowAssets,
                badDebtShares.toAssetsUp(market[id].totalBorrowAssets, market[id].totalBorrowShares)
            );

            market[id].totalBorrowAssets -= badDebt.toUint128();
            market[id].totalSupplyAssets -= badDebt.toUint128();
            market[id].totalBorrowShares -= badDebtShares.toUint128();
            position[id][borrower].borrowShares = 0;
        }

        IERC20(marketParams.collateralToken).safeTransfer(msg.sender, seizedAssets);

        // `repaidAssets` may be greater than `totalBorrowAssets` by 1.
        emit EventsLib.Liquidate(id, msg.sender, borrower, repaidAssets, repaidShares, seizedAssets, badDebtShares);

        if (data.length > 0) IMorphoLiquidateCallback(msg.sender).onMorphoLiquidate(repaidAssets, data);

        IERC20(marketParams.loanToken).safeTransferFrom(msg.sender, address(this), repaidAssets);

        return (seizedAssets, repaidAssets);
    }

The liquidator can liquidate the entire collateral without clearing the debt by just paying one share at a time.

The exchange rate of shares to asset is:

$$Exchange Rate = \cfrac{TotalAssets}{TotalShares} = \frac{3}{3e6} = 0.000001 = 1e-6 \\$$

$$\text{Which means 1 share = 0.000001 asset}$$

Suppose Alice position is the only open borrow position in the market.

$$RepaidAssets = \frac{Shares * TotalAssets + Virtual Asset + (TotalShares + Virtual share-1)} {TotalShares + Virtual Shares}$$

$$RepaidAssets = \frac{1 * 3 + 1 + (3e6 + 1e6 - 1) )}{(3e6 + 1e6)} = \frac{3 + 4e6}{4e6} = 1$$

Note: EVM rounds down to zero every number after the decimal

The repaid assets is then used to calculate the seizedAssets as seen in the code above. Remember the exchange rate is 1 share to 0.000001 , but in the calculation, it is rounded up to 1 asset. Meaning 1 - 0.000001 will be wrongly seized from Alice position, there by leaving her with less collateral and her position less healthy than before. The liquidator can perform this action multiple times until the collateral left is zero.

Virtual borrow shares accrue interest and lead to bad debt #450

Author: cmichel

Virtual shares as designed in Blue, are accounted for each time an external balance changing event occurs withdrawsupplyborrowrepayliquidate . which is essentially changing assets to shares and shares to assets.

function toSharesDown(uint256 assets, uint256 totalAssets, uint256 totalShares) internal pure returns (uint256) {
    return assets.mulDivDown(totalShares + VIRTUAL_SHARES, totalAssets + VIRTUAL_ASSETS);
}

/// @dev Calculates the value of `shares` quoted in assets, rounding down.

function toAssetsDown(uint256 shares, uint256 totalAssets, uint256 totalShares) internal pure returns (uint256) {
    return shares.mulDivDown(totalAssets + VIRTUAL_ASSETS, totalShares + VIRTUAL_SHARES);
}

/// @dev Calculates the value of `assets` quoted in shares, rounding up.

function toSharesUp(uint256 assets, uint256 totalAssets, uint256 totalShares) internal pure returns (uint256) {
    return assets.mulDivUp(totalShares + VIRTUAL_SHARES, totalAssets + VIRTUAL_ASSETS);
}

/// @dev Calculates the value of `shares` quoted in assets, rounding up.

function toAssetsUp(uint256 shares, uint256 totalAssets, uint256 totalShares) internal pure returns (uint256) {
    return shares.mulDivUp(totalAssets + VIRTUAL_ASSETS, totalShares + VIRTUAL_SHARES);
}

$$SharesToAssetUp = \frac{ Shares * TotalAssets + Virtual Asset + (TotalShares + Virtual Shares - 1)}{TotalShares + virtual Shares}$$

$$SharesToAssetDown = \frac{Shares * TotalAssets + Virtual Asset}{TotalShares + virtual Shares}$$

$$AssetToSharesUp = \frac{ Assets * TotalShares + Virtual Shares + (TotalAssets + Virtual Assets -1)}{TotalAssets + Virtual Assets}$$

$$AssetToSharesDown = \frac{Assets * TotalShares + Virtual Shares}{TotalAssets + Virtual Assets}$$

The above are formulas for the calculation to convert asset to shares and shares to assets that presents itself in all entry and exit functions as listed above. As can be seen the virtual shares/assets are present in all the formulas.

function _accrueInterest(MarketParams memory marketParams, Id id) internal {
    uint256 elapsed = block.timestamp - market[id].lastUpdate;
    if (elapsed == 0) return;
    uint256 borrowRate = IIrm(marketParams.irm).borrowRate(marketParams, market[id]);
    uint256 interest = market[id].totalBorrowAssets.wMulDown(borrowRate.wTaylorCompounded(elapsed));
    market[id].totalBorrowAssets += interest.toUint128();
    market[id].totalSupplyAssets += interest.toUint128();
    --------------------------------
}

When the market accrues interest as in the above, it then means that each share accrues debt in the totalBorrowShares, that no one is going to pay as it does not belong to anyone, which in turn leads to bad debt.

Comments:

cergyk: ..... Since the interest accumulated by virtual debt shares is at worst the interest incurred by the 1 wei of virtual debt assets at inception, this report should be invalid.

cmichel: ..... Impact low: It's just 1e6 virtual borrow shares earning interest on 1 wei, so the bad debt accrued is negligible even for very high borrow rates. Likelhood high: The virtual debt shares exist in every market, it's an issue in every market.

Suppliers can be tricked into supplying more Assets #314

Author: cmichel

function supply(
    MarketParams memory marketParams,
    uint256 assets,
    uint256 shares,
    address onBehalf,
    bytes calldata data

) external returns (uint256, uint256) {

    Id id = marketParams.id();
    require(market[id].lastUpdate != 0, ErrorsLib.MARKET_NOT_CREATED);
    require(UtilsLib.exactlyOneZero(assets, shares), ErrorsLib.INCONSISTENT_INPUT);
    require(onBehalf != address(0), ErrorsLib.ZERO_ADDRESS);

    _accrueInterest(marketParams, id);

    if (assets > 0) shares = assets.toSharesDown(market[id].totalSupplyAssets, market[id].totalSupplyShares);
    else assets = shares.toAssetsUp(market[id].totalSupplyAssets, market[id].totalSupplyShares);

    position[id][onBehalf].supplyShares += shares;
    market[id].totalSupplyShares += shares.toUint128();
    market[id].totalSupplyAssets += assets.toUint128();

    emit EventsLib.Supply(id, msg.sender, onBehalf, assets, shares);

    if (data.length > 0) IMorphoSupplyCallback(msg.sender).onMorphoSupply(assets, data);

    IERC20(marketParams.loanToken).safeTransferFrom(msg.sender, address(this), assets);

    return (assets, shares);
}

When supplying Assets to a market, the supplier has two options to supply through, either through shares or through assets, but never both. The possible bug is when supplies are made through shares. A malicious user can make the supplier supply more than intended in cases where the supplier approves max to the contract on the selected token. This is especially possible since the morpho blue contract is a singleton contract, a single contract which has all markets, so the user might have approved max when using a different market on same token. A malicious user can in effect use the withdraw function as a burn, this is very possible after market creation, if the malicious user tries to withdraw a share size less than 1 asset like so: Since 1 asset gives your 1e16 share, the malicious user inputs (1e6 - 1) which should give you a share value less than 1 asset, but since EVM does not support decimals, its rounded to zero, hence meaning the malicious user just burnt the shares and inflated the cost of shares in the market by so doing so.

POC

1. A new market is created.

2. The victim tries to supply 1 assets at the initial share price of 1e-6 and specifies supply(shares=1e6). They have already given max approval to the contract because they already supplied the same asset to another market.

3. The attacker wants to borrow a lot of the loan token and therefore targets the victim. They front run the victim by supply(assets=100) and a sequence of withdraw() functions such that totalSupplyShares = 0 and totalSupplyAssets = 100. The new supply share price increased 100x.

4. The victim's transaction is minted and they use the new supply share price and mint 100x more tokens than intended. (Possible because of the max approval.)

5. The attacker borrows all the assets.

6. The victim is temporarily locked out of that asset. They cannot withdraw again because of the liquidity crunch (it is borrowed by the attacker).