Description

To start, there are no timelocks on the senior and junior depositor vaults. Furthermore, the share of vault assets lenders are entitled to when they withdraw is based on the share of assets at the time of deposit:

function pushWithdraw(address _user, uint256 _shares) internal {
    unbondings[_user].shares += _shares;
    unbondings[_user].maxUnderlying += convertToAssets(_shares);
    totalUnbonding += _shares;
}

And these funds are held in bonding until they are claimed:

function claim() external {
    uint256 maxClaimable = unbondings[msg.sender].maxUnderlying;
    [...]

This means a malicious user can potentially steal an outsized share of the principal and interest payments by manipulating their vault shares through deposits, withdraws, and claims.

Impact

Given that a lender can also purchase an NFT, the above opens up a novel approach for NFT theft as follows:

1. Take out a flash loan.

2. Call deposit(...) - make a large vault deposit and get awarded an outsized number of shares (shares are proportional to total asset share).

3. Call buyNFT(...) - purchase an NFT by making the first principal and interest payments.

4. Call withDraw(...) - with a large enough flash loan, you should be able to lock for withdraw the majority of the principal and interest payments you just made.

5. Call claim(...) - remove your funds from the vault and pay back your flash loan. You will need a separate source of funds to pay interest on the flash loan (longer-term loan, whale).

6. Repeat steps 2, 4, and 5 until the maturity of the loan has passed.

7. Call withdrawNFT(...) to take posession of your NFT. Sell it and repay any outstanding debts.

Recommendations

The vector above can be blocked by preventing lenders from also purchasing NFTs; however, this would be a naive fix. The ability to deposit and withdraw funds without timelocks in order to create a maxClaimable slip that can be used to claim interest and principal payments at any time is a fundamental design flaw. It means depositors can game the system, claiming principal and interest payments for which they hold no credit risk.

We suggest implementing a timelock mechanism on depositors' shares to ensure they are "paying their dues." This will help ensure depositors take on levels of credit risk commensurate with their returns. It is true that depositors who come in at later dates may end up covering losses on assets lent out earlier; our interpretation is that this is part of the pooling design. However, we feel the ability to game this exposure is a design flaw and should be removed.

Remediation

Commit a5bfd675↗ was indicated as containing the remediation. Rewiewing the remediation for this issue has proven to be challenging due to the pace of the development. A total of 181 commits exist between the commit under review and a5bfd675↗; the diff between the two commits amounts to 43 solidity files changed, with 2416 insertions and 1684 deletions. The issue appears to be correctly fixed at the given commit. We largely based this evaluation on the description provided by the Voyage team due to the considerable amount of changes, which aligns with what can be observed in the commits. In particular, the code tracking the balances of the amounts deposited by the users has been updated to keep track of the unbonding amounts; further, we observed no anomalies in the evolution of the balances during the execution of a proof of concept developed to demonstrate the issue when executed against the commit containing the remediation.

We note that it is still technically possible to reclaim a disproportionate amount of the interests portion of the installments by depositing a very large amount of assets before buying an NFT and withdrawing after repayments are made. The Voyage team has argued that this strategy does not seem to be exploitable to gather a profit. Their assessment is likely to be correct for the economic conditions in which Voyage is expected to operate, although profitability might be possible if some parameters such as flash loan interest rates, Voyage pool asset sizes and NFT values were to assume unexpected values.