Maple Finance audit
A private security review of Maple Finance (institutional lending), conducted with Three Sigma and completed on 10th April 2024 over 12 days. 0xSimao disclosed 35 findings: 6 high severity, 6 medium, 9 low and 14 informational.
What Maple Finance is
Maple Finance runs institutional lending pools on Ethereum. Lenders deposit into ERC-4626-style pools, a pool delegate underwrites borrowers and deploys the capital into fixed-term and open-term loans, and first-loss cover staked by the delegate absorbs defaults ahead of lenders. Withdrawals are not a simple redeem: they queue in a withdrawal manager and settle against the liquidity the pool actually holds.
The system is built as a constellation of small, proxied contracts: MapleGlobals holds protocol-wide authority and timelocks, PoolManager wires each pool to its loan managers, fee manager and withdrawal manager, and separate loan-manager implementations track fixed-term and open-term debt with their own interest accrual and impairment logic. Nearly every privileged action flows through governor or delegate roles checked against Globals, so the trust model is as much a part of the code as the accounting.
Scope
The Three Sigma review covered the full loan and pool stack: the fixed-term and open-term loan contracts, the loan managers, the pool and its withdrawal manager, the fee manager and the protocol globals.
High severity findings
- Fixed term loans can be deployed with a wrong fee manager and possibly steal all funds.
- Pool Delegates can set a really high origination fee and steal all pool funds.
- Pool Delegates can steal all the pool's funds by setting a malicious Withdrawal Manager.
- Pool Delegates can set an unreasonably large delegate management fee rate at anytime
- Liquidation can be finished without calling triggerDefault() (and repossessing the loan) if pool delegate or governor call finishCollateralLiquidation(...) after impairment.
- Malicious refinancer danger due to fixed term loan upgrades.
Medium severity findings
- Open term loans can be created with zero platform service fee if borrowers create them right after a pool has been deployed.
- fundLoan() can be DoSed if returnFunds() is called before it.
- A pool delegate should not be allowed to be a borrower
- MapleGlobals, activatePoolManager() has no check that the pool manager is actually a valid pool manager.
- DoS attack if assets are transferred before a fundLoan() call.
- Pool Delegates can accidentally lock out of their funds LPs in the middle of redeeming.
Low severity findings
- MapleLoan, proposeNewTerms() could have a check for duplicate selectors.
- MapleLoan, skim() has no zero address transfer check.
- In MapleLoanInitializer, the Borrower can choose a different fundsAsset than the lender.
- Borrowers can prevent MapleLoanInitializer from initializing a loan by using a vanity address.
- Consider using a time lock on critical permissioned functions.
- In MapleGlobals, there is no check in the dataHash argument when unscheduling a call.
- Consider adding a check to ensure that fees, delegate cover and so on have been explicitly set.
- In PoolManager, setOpenToPublic(), there is no way to unset openToPublic.
- Throughout code-base: missing address checks.
Informational findings
- MapleLoan variables read from storage more than once.
- MapleLoanStorage storage slot optimization.
- MapleLoan change the order of require to save on gas.
- In PoolManager, requestFunds(...) repeated variable fetching from storage.
- In MapleLoanFactory, isLoan has the same functionality of isInstance and thus can be removed.
- LoanManager code optimizations: no need to load payment struct from storage if loan is unimpaired.
- LoanManager code optimizations: redundant impairment functions.
- Throughout code base: use Solidity native errors implementation instead of string errors.
- open-term-loan-private: check dateFunded!=0 first to save gas.
- pool-v2-private: cache list length in memory and uncheck i_ to save gas.
- Throughout code-base: Homogenize how access control is done.
- Missing documentation for MapleLoan, SetPendingLender and AcceptLender being implemented on MapleLoan but not on LoanManager.
- Multiple documentation fixes throughout the repository.
- globals-v2-private: typo in function name.
The report