Gondi Mitigation Review details

• Total Prize Pool: $16,400 in USDC
  • HM awards: $13,140 in USDC
  • Judge awards: $1,460 in USDC
• Warden guidelines for C4 mitigation reviews
• Submit findings using the C4 form
• Starts May 14, 2024 20:00 UTC
• Ends May 24, 2024 20:00 UTC

Important note

Each warden must submit a mitigation review for every individual PR listed in the Scope section below. Incomplete mitigation reviews will not be eligible for awards.

Findings being mitigated

Mitigations of all High and Medium issues will be considered in-scope and listed here.

Highs

• H-01: Merging tranches could make _loanTermination() accounting incorrect
• H-02: Division before multiplication could lead to users losing 50% in WithdrawalQueue
• H-03: Function distribute() lacks access control allowing anyone to spam and disrupt the pool's accounting
• H-04: Function refinanceFromLoanExecutionData() does not check executionData.tokenId == loan.nftCollateralTokenId
• H-05: triggerFee is stolen from other auctions during settleWithBuyout()
• H-06: Function settleWithBuyout() does not call LoanManager.loanLiquidation() during a buyout
• H-07: deployWithdrawalQueue() need clear _queueAccounting...
• H-08: Incorrect circular array check in _updatePendingWithdrawalWithQueue flow , causing received funds added to the wrong queues
• H-09: Incorrect accounting of _pendingWithdrawal in queueClaiming flow
• H-10: The attackers front-running repayloans so that the debt cannot be repaid
• H-11: Incorrect protocol fee implementation results in outstandingValues to be mis-accounted in Pool.sol
• H-12: addNewTranche() no authorization from borrower
• H-13: _processOffersFromExecutionData() lack of check executionData.duration<=offer.duration
• H-14: mergeTranches()/refinancePartial() lack of nonReentrant
• H-15: _baseLoanChecks() check errors for expire
• H-16: validateOffer() reentry to manipulate exchangeRate
• H-17: refinanceFull/addNewTranche reusing a lender's signature leads to unintended behavior

Mediums

• M-01: Invalid maxTranches check can result in maxTranche cap to be exceeded
• M-02: A malicious user can take on a loan using an existing borrower's collateral in refinanceFromLoanExecutionData()
• M-03: Function addNewTranche() should use protocolFee from Loan struct
• M-04: Function Pool.validateOffer() does not work correctly in case principalAmount > currentBalance
• M-05: Collected fees are never transferred out of Pool contract
• M-06: Anyone can remove existing term without queueing through setTerms()
• M-07: Attacker can front-run and pass in empty terms, making it impossible to confirmTerms()
• M-08: Borrower signature could be reused in emitLoan()
• M-09: Inconsistent accounting of undeployedAssets might result in undesired optimal range in the pool
• M-10: Any liquidators can pretend to be a loan contract to validate offers, due to insufficient validation
• M-11: AuctionLoanLiquidator#placeBid can be DoS
• M-12: Pool.getMinTimeBetweenWithdrawalQueues current calculations may not be sufficient
• M-13: confirmBaseInterestAllocator() change BaseInterestAllocator may pay large getReallocationBonus
• M-14: loanLiquidation() calculation of interest is not accurate
• M-15: confirmUnderwriter() need to recalculate getMinTimeBetweenWithdrawalQueues
• M-16: distribute() Use the wrong end time to break maxSeniorRepayment's expectations
• M-17: loan.hash() does not contain protocolFee
• M-18: distribute() when can't repay all lenders, may lack of notification to LoanManager for accounting
• M-19: Bidders might lose funds due to possible racing condition between settleWithBuyout and placeBid
• M-20: Hardcoded incorrect getLidoData timestamp, resulting in incorrect base point Apr. Loans can be validated with a substantially low baseRate interest

Overview of changes

All invidivual fixes are addressed in a separate PR. The branche feature/developWithOpt contains the merge of all such features + a few that were found by other auditors. Because of contract size issues when adding all fixes, the following has changed:

• There's no bonus on Pool reallocation (we expect the dao to run this function and incur the cost)
• In refinancePartial, we don't allow extra principal (this is a corner case and can still be done with addNewTranche)
• Some of the two step variable changes in the Pool (LoanManager) have been moved to a helper contract.
  Feel free to ignore issues regarding the PoolOfferHandler since we are working on a different one given the limitations of the existing one.
• Please note the out-of-scope item H-10. While not in scope for awards, general commentary on its design is welcome. The PR for H-12 can give some insight into it.

Scope

Branch

https://github.com/pixeldaogg/florida-contracts/pull/394

Mitigation of High & Medium Severity Issues

Out of Scope

H-10 - Refinacing a loan locks the loan. Adding a tranche can only be accepted by the borrower now (H-10). External actors can only front-run a limited number of times.