Platform: Code4rena
Start Date: 05/04/2022
Pot Size: $30,000 USDC
Total HM: 10
Participants: 47
Period: 3 days
Judge: gzeon
Total Solo HM: 4
Id: 106
League: ETH
Rank: 5/47
Findings: 3
Award: $1,132.96
🌟 Selected for report: 0
🚀 Solo Findings: 0
829.5088 USDC - $829.51
NFTLoandFacilitator.lend; L205
A lender using an upgradeable smart contract could stop other lenders from buying him out, essentially causing a DoS.
Lender uses an upgradeable smart contract that front runs competing lenders and upgrades itself to remove its fallback/receive function everytime another lender tries to buy him out. Causing a DoS when the lend function reaches L205
Then, before he is about to be repayed he front runs repayAndCloseLoan and upgrades itself again to be able to receive the rewards.
Ensure lenders can only be EOAs.
#0 - wilsoncusack
2022-04-08T01:02:38Z
Proposed resolution does not fix because even if original lender is an EOA they can transfer the ticket to a smart contract. Won't fix. Only so much we can do with malicious tokens
#1 - gzeoneth
2022-04-15T13:27:27Z
Duplicate #89
#2 - wilsoncusack
2022-04-18T22:55:31Z
Hey sorry @gzeoneth just looking closer at this one: I would argue not a duplicate of ERC777? Borrower could be using an normal ERC777 and be attacked in #89. This requires borrower opting into using malicious ERC20
#3 - wilsoncusack
2022-04-18T23:06:41Z
I would have marked this as invalid
#4 - wilsoncusack
2022-04-18T23:09:03Z
actually: I think I am mistaken in my reading here. It is correctly marked as a duplicate of #89
🌟 Selected for report: WatchPug
Also found by: Dravee, berndartmueller, hake, jah, minhquanym
181.4136 USDC - $181.41
ERC721 used as collateral could possibly never return to borrower.
No zero address check for sendCollateralTo might lead to sending ERC721 used as collateral to inexistent address.
Use of transferFrom instead of safeTransferFrom doesn't check if ERC721 has been properly received.
Allowing users to send their tokens to oblivion (even if by mistake) can cause major reputation damage, given such tokens might be extremely valuable.
Implement zero address check and/or use safeTransferFrom instead of transferFrom.
#0 - wilsoncusack
2022-04-07T23:44:29Z
ERC721 standard states that transferFrom Throws if _to is the zero address, so 0 address is checked.
This has been brought up in several tickets and I mentioned that we feel users passing custom params here should know what they are doing and are not going to have the gas overhead of safeTransferFrom. Mentioned elsewhere the even uniswap uses _mint rather than _safeMint https://github.com/Uniswap/v3-periphery/blob/main/contracts/NonfungiblePositionManager.sol#L156
#1 - gzeoneth
2022-04-15T13:34:49Z
Duplicate of #83
🌟 Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0xDjango, 0xkatana, BouSalman, CertoraInc, Dravee, FSchmoede, Hawkeye, Kenshin, Meta0xNull, PPrieditis, Ruhum, TerrierLover, VAD37, WatchPug, berndartmueller, csanuragjain, hake, horsefacts, hubble, m9800, rayn, reassor, robee, samruna, securerodd, shenwilly, sorrynotsorry, t11s, teryanarmen, tintin, z3s
122.0352 USDC - $122.04
withdrawOriginationFees could be better implemented.withdrawOriginationFees; L295-300
Balance in the contract might differ from balance generated by loan origination fees for whatever reason.
Maybe implementing origination fee balance accounting could improve accuracy of this function.
Furthermore, a zero address check should be implemented for to to make sure the origination fees are not burned.
_safeMint instead of _mint.NFTLoanTicket.mint; L33-35
Recommend implementing _safeMint instead of _mint to ensure tokens have been properly received.
setLendTicketContract and setBorrowTicketContractsetLendTicketContract setBorrowTicketContract
Zero address checks should be implemented in these functions to avoid having to relaunch NFTLoandFacilitator in case one of them is set to 0 by mistake.
updateRequiredImprovementRateupdateRequiredImprovementRate; L314-326
RequiredImprovementRate could be set by borrower. Small percentages can still make a big difference if big figures are in play. Borrower could appreciate more flexibility in this area.
Merely an opinion.