Description

Calls to liquidate(...) made when the discounted price of the underlying NFT is greater than the price paid when buying it will move funds from junior depositors and send them to senior depositors. In liquidate(...),

param.remaningDebt = param.totalDebt;
// [...]
param.receivedAmount = discountedFloorPriceInTotal;
// [...]
if (param.totalDebt > discountedFloorPriceInTotal) {
    param.remaningDebt = param.totalDebt - discountedFloorPriceInTotal;
} else {
    uint256 refundAmount = discountedFloorPriceInTotal -
        param.totalDebt;
    IERC20(param.currency).transfer(param.vault, refundAmount);
    param.receivedAmount -= refundAmount;
}

If param.totalDebt > discountedFloorPriceInTotal, then param.receivedAmount = param.totalDebt and param.remaningDebt = param.totalDebt. The following code will therefore execute the following:

if (param.remaningDebt > 0) {
    param.totalAssetFromJuniorTranche = ERC4626(
        reserveData.juniorDepositTokenAddress
    ).totalAssets();

    if (param.totalAssetFromJuniorTranche >= param.remaningDebt) {
        IVToken(reserveData.juniorDepositTokenAddress)
            .transferUnderlyingTo(address(this), param.remaningDebt);
        param.juniorTrancheAmount = param.remaningDebt;
        param.receivedAmount += param.remaningDebt;
    } else {
        IVToken(reserveData.juniorDepositTokenAddress)
            .transferUnderlyingTo(
                address(this),
                param.totalAssetFromJuniorTranche
            );
        param.juniorTrancheAmount = param.totalAssetFromJuniorTranche;
        param.receivedAmount += param.totalAssetFromJuniorTranche;
        param.writeDownAmount =
            param.remaningDebt -
            param.totalAssetFromJuniorTranche;
    }
}

It can be verified that param.receivedAmount = 2 * param.totalDebt or param.receivedAmount = param.totalDebt + param.totalAssetFromJuniorTranche depending on whether param.totalAssetFromJuniorTranche >= param.remaningDebt.

Voyage will be in possession of assets equal to param.receivedAmount; furthermore, param.receivedAmount will be sent to the senior depositors:

IERC20(param.currency).safeTransfer(
    reserveData.seniorDepositTokenAddress,
    param.receivedAmount
);

Impact

The finding has been rated as critical because it could have catastrophic consequences for the performance of the protocol.

1. Junior depositors would be missing funds with potentially no explanation. This is likely to be realized by users over time and may result in near complete abandonment of the junior tranche and hence loss of core protocol functionality and purpose.

2. It would raise the prospect of additional yet unfound issues that could end up affecting senior depositors. This could result in a complete loss of confidence in the project and team.

Recommendations

Make the following code change in liquidate(...):

 } else {
     uint256 refundAmount = discountedFloorPriceInTotal -
         param.totalDebt;
     IERC20(param.currency).transfer(param.vault, refundAmount);
     param.receivedAmount -= refundAmount;
+    param.remaningDebt = 0;
}

Remediation

Voyage has since made considerable changes to the code base in order to fundamentally alter the way funds are distributed in the event of liquidations. We view the changes to the code base as extending beyond remediation efforts targeting the basic coding mistake we have identified and as constituting extensions to the code base that would require extending the scope of the audit engagement. For context, there have been 81 commits made since the scoping of the audit and this remediation commit provided by Voyage 654a9242↗. Across these commits a total of 30 solidity files have been changed with a total of 1,406 insertions and 1008 deletions. Out of respect for the scope of the initial engagement we have not been able to fully audit these changes and confirm whether the underlying issue identified here has indeed been remediated. However, we can confirm that Voyage has acknowledged the issue and has claimed to have fixed it in these architectural changes.