Omniscia Arcade XYZ Audit

RepaymentController Manual Review Findings

RepaymentController Manual Review Findings

RCR-01M: Improper Imposition of Claim Fee

Description:

The RepaymentController::claim function will impose a claimFee to the lender that needs to be paid immediately before unlocking the NFT asset.

This approach is counter-intuitive as the lender may wish to liquidate the asset to cover their other obligations, meaning that they may not have the funds available to repay the claimFee before unlocking the NFT.

This would be especially prevalent in high-value loans that use f.e. CryptoPunks as the claimFee is proportionate.

Impact:

Imposing a claim fee on a loan that has entered default is an ill-advised trait of the protocol as it indicates an edge case whereby the lender has potentially already incurred a loss.

Example:

contracts/RepaymentController.sol
102function claim(uint256 loanId) external override {
103    LoanLibrary.LoanData memory data = loanCore.getLoan(loanId);
104    if (data.state == LoanLibrary.LoanState.DUMMY_DO_NOT_USE) revert RC_CannotDereference(loanId);
105

106    // make sure that caller owns lender note
107    // Implicitly checks if loan is active - if inactive, note will not exist
108    address lender = lenderNote.ownerOf(loanId);
109    if (lender != msg.sender) revert RC_OnlyLender(lender, msg.sender);
110

111    LoanLibrary.LoanTerms memory terms = data.terms;
112    uint256 interest = getInterestAmount(terms.principal, terms.proratedInterestRate);
113    uint256 totalOwed = terms.principal + interest;
114

115    uint256 claimFee = (totalOwed * data.feeSnapshot.lenderDefaultFee) / BASIS_POINTS_DENOMINATOR;
116

117    loanCore.claim(loanId, claimFee);
118}

Recommendation:

We advise the claimFee to either be removed, be a fixed amount, or to be due after the NFT has been claimed. The latter of the three options is already achievable due to the usage of the re-entrant EIP-721 ERC721::safeTransferFrom function via the LoanCore::claim function.

Alleviation (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):

The Arcade XYZ team re-ordered the operations performed by the LoanCore::claim function to collect the claim fee from the lender after the EIP-721 asset has been transferred to them, alleviating this exhibit in full.

RCR-02M: Inexistent Validation of Loan Expiry

Description:

The RepaymentController::repay and RepaymentController::forceRepay functions as well as the LoanCore::repay and LoanCore::forceRepay functions do not validate whether a loan has expired.

Impact:

The current implementation permits technically-savvy individuals to create bots that would transmit a repayment transaction solely when they detect a RepaymentController::claim operation by the lender. As they are able to repay at any time, they can cause all RepaymentController::claim operations to fail.

Example:

contracts/RepaymentController.sol
65/**
66 * @notice Repay an active loan, referenced by borrower note ID (equivalent to loan ID). The interest for a loan
67 *         is calculated, and the principal plus interest is withdrawn from the caller.
68 *         Anyone can repay a loan. Control is passed to LoanCore to complete repayment.
69 *
70 * @param  loanId               The ID of the loan.
71 */
72function repay(uint256 loanId) external override {
73    (uint256 amountFromBorrower, uint256 amountToLender) = _prepareRepay(loanId);
74

75    // call repay function in loan core -  msg.sender will pay the amountFromBorrower
76    loanCore.repay(loanId, msg.sender, amountFromBorrower, amountToLender);
77}
78

79/**
80 * @notice Repay an active loan, referenced by borrower note ID (equivalent to loan ID). The interest for a loan
81 *         is calculated, and the principal plus interest is withdrawn from the caller. Anyone can repay a loan.
82 *         Using forceRepay will not send funds to the lender: instead, those funds will be made
83 *         available for withdrawal in LoanCore. Can be used in cases where a borrower has funds to repay
84 *         but the lender is not able to receive those tokens (e.g. token blacklist).
85 *
86 * @param  loanId               The ID of the loan.
87 */
88function forceRepay(uint256 loanId) external override {
89    (uint256 amountFromBorrower, uint256 amountToLender) = _prepareRepay(loanId);
90

91    // call repay function in loan core -  msg.sender will pay the amountFromBorrower
92    loanCore.forceRepay(loanId, msg.sender, amountFromBorrower, amountToLender);
93}

Recommendation:

Either the LoanCore or the RepaymentController system should prohibit repayments from occurring if the loan's dueDate as calculated within LoanCore::claim has surpassed.

Alleviation (7a4e1dc948e94ded7385dbb74818bcf93ecc207c):

The Arcade XYZ team stated that repayments of expired loans are expected to be possible as the onus of liquidating / defaulting a loan is up to the lender akin to other lending protocols such as Aave.

As such, we consider this exhibit nullified given that it represents desirable behaviour by the Arcade XYZ team.

Prev

PunksVerifier.sol (PVR-M)

Next

ArcadeItemsVerifier.sol (AIV-C)

On this page