Description

The price oracle stores the average price of NFTs in a given collection. Calls to updateTWAP(...) set the average price and the block.timestamp:

function updateTwap(address _currency, uint256 _priceAverage)
    external
    auth
{
    prices[_currency].priceAverage = _priceAverage;
    prices[_currency].blockTimestamp = block.timestamp;
}

The timestamp is returned from calls to getTWAP(...):

function getTwap(address _currency)
    external
    view
    returns (uint256, uint256)
{
    return (
        prices[_currency].priceAverage,
        prices[_currency].blockTimestamp
    );
}

Unfortunately, the time stamp is never used in buyNow(...).

Since the protocol expects only NFTs satisfying the following two conditions to be purchased,

if (params.fv == 0) {
    revert InvalidFloorPrice();
}

if (params.fv < params.totalPrincipal) {
    revert InvalidPrincipal();
}

an out-of-date price oracle means these conditions could be violated.

Furthermore, there are no stale price checks in liquidate(…):

IPriceOracle priceOracle = IPriceOracle(
    reserveData.priceOracle.implementation()
);
(param.floorPrice, param.floorPriceTime) = priceOracle.getTwap(
    param.collection
);
if (param.floorPrice == 0) {
    revert InvalidFloorPrice();
}
[...]
param.totalDebt = param.principal;
param.remaningDebt = param.totalDebt;
param.discount = getDiscount(param.floorPrice, param.liquidationBonus);
param.discountedFloorPrice = param.floorPrice - param.discount;


uint256 discountedFloorPriceInTotal = param.discountedFloorPrice *
    collaterals.length;
IERC20(param.currency).safeTransferFrom(
    param.liquidator,
    address(this),
    discountedFloorPriceInTotal
);

Impact

If an NFT was purchased with a price greater than the average price (i.e., params.fv < params.totalPrincipal), then lenders may end up backing much riskier assets than intended. Additionally, if stale prices are below current prices a liquidator would be able to purchase the NFTs at a discount and sell them for a profit. On the other hand, if stale prices were above market prices, NFTs could stay locked in the system.

The utility of credit products for users depends immensely on good alignment between the underlying credit dynamics and user expectations. This logic error can result in a rate of loan defaults that is largely outsized to investor expectations.

Additionally, upon liquidation it can also result in vault loss of funds through selling NFTs at submarket prices.

Recommendations

Introduce stale price checks in buyNow(...) and liquidate(..). The protocol operators need to determine the appropriate length of the time window to be accepted for the last average price. Because these are NFT markets, it is important to ensure the window is long enough so that it reflects a sufficient number of trades while at the same time not including out-of-date trades.

Remediation

Voyage has introduced stale price checks in both buyNow(...) and liquidate(...) in the following commits 80a681a2↗ and 654a9242↗.