NettyWorth

After a loan is created, a NFT that is stored as collateral in the Vault can only be obtained by the lender or borrower after the loan is finished or liquidated.However, it is possible for an unauthorized user to obtain all the nft from the vault. This can be done because the safeBatchTransfer() function allows any user to call it as it is defined as public.

PromissoryReceipt can be transferred using the transferPromissoryReceipt() function but this will cause a problem in the protocol as the updateLender() function from LoanManager does not update the _assetsHash neither _assets maps from the CryptoVault contract.This prevents liquidation of a loan whose promissory receipt has been transferred by using the correct transferPromissoryReceipt() function.

PromissoryReceipt and ObligationReceipt can be transferred using the basic functions for transferring NFTs such as safeTransferFrom() or transferFrom() but this will cause a problem in the protocol as it does not update the borrower or lender fields in the LoanManager contract.This prevents liquidation or repayment of a loan whose any of its receipts have been transferred by using any of the methods mentioned above.

The protocol allows a loan that has been finalized to be repaid as long as it is not liquidated. This makes the interest and the fees incorrect.The borrower could wait until lender calls forCloseLoan() function to frontrun him and close the loan by calling payBackLoan() and obtain a loan for a longer period than initially stated.

When a loan is created, both parties agree on the loan duration. The protocol implements mechanisms to avoid lenders to liquidate the loan before it ends but it is still possible for lenders to liquidate a loan in the last block of the loan period.The forCloseLoan() function should check if the block.timestamp is greater than loan.loanDuration instead of checking if it is greater or equal.This may result in borrowers potentially losing the collateral NFT at the last timestamp of the loan's duration.

The createLoan() function allows the creation of a loan with an _aprBasisPoints higher than 100%.When a borrower tries to payback a loan with this huge rate, the require that checks the loan.aprBasisPoints in the getPayoffAmount() function will revert.

When a type is downcast to a smaller type, the higher order bits are truncated, effectively applying a modulo to the original value. Without any other checks, this wrapping will lead to unexpected behavior and bugs.In this protocol, failures caused by collisions could lead to errors in loan creation.

When it is not possible for counters to overflow, as is the case when used in for and while-loops, they should be unchecked. The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop.

Unless there is a compelling reason, abi.encode() should be preferred instead of abi.encodePacked().abi.encode() will pad parameters to 32 bytes, which will prevent hash collisions.For example, abi.encodePacked(0x123,0x456) returns 0x123456 which will collide with abi.encodePacked(0x1,0x23456). but abi.encode(0x123,0x456) will result in 0x0...1230...456If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.If all arguments are strings and or bytes, bytes.concat() should be used instead.It should be noted that the severity of this finding has been decreased since no dynamic types are being used inside the function abi.encodePacked().

The updateAdminFee() function allows an owner to set a fee of 100% which may not be practical or could be considered predatory lending.