Maple Finance - Maple Core
Smart Contract Security Assessment
Mar. 12, 2021
SUMMARY
ABSTRACT
Dedaub was commissioned to perform a security audit on Maple Finance’s core smart contracts (maple-core). The audit was performed on commit 05ef95f with revisions subsequently checked against the issues identified.
Four auditors worked on the task over the course of 8 working days. We reviewed the code in significant depth, assessed the economics of the protocol and processed it through automated tools. We also decompiled the code and analyzed it, using our static analysis (incl. symbolic execution) tools, to detect possible issues.
SETTING AND CAVEATS
The code base is medium-size, at around 3.5KLoC (non-test, non-interface files). However, the economic mechanisms behind the Maple Core protocol are complicated. We also account for protocol composability issues and the economic risks these bring in this audit.
The audit focused on security, establishing the overall security model and its robustness and also crypto-economic issues. Functional correctness (e.g., that the calculations are correct) was a secondary priority. Functional correctness relative to low-level calculations (e.g., units) is generally most effectively done through thorough testing.
Revisions post-audited-commit were inspected to the extent that they addressed issues identified in this report. The development team should be aware that such revisions are audited with the auditor having less context than during the original code audit. Therefore post-audited-commit revisions are expected to be made with extreme care, relative to functionality unrelated to the flagged issue that the revision intends to address.
Trust Model/Centralization Elements
[This section is included for context, although its contents should already be known to the commissioner of an audit.]
There are significant centralization elements, but these are in accordance with a service that offers under-collateralized loans. Much of the trust in the protocol is derived in the off-chain world. E.g., users need to trust the protocol owner account and the vetting of loans performed by the pool delegate. Borrowers are trusted because of real-world reputation and potential consequences. A notable centralization element, however, is that, in this version of the code, there is significant trust in the pool delegates. Pool delegates can subvert many of the checks and balances if they act maliciously. For instance, there are at least two different flash-loan-based attacks outlined below (present in the final, revised version of the code) that pool delegates can perform.
VULNERABILITIES & FUNCTIONAL ISSUES
This section details issues that affect the functionality of the contracts. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:
- User or system funds can be lost when third-party systems misbehave.
- DoS, under specific conditions.
- Part of the functionality becomes unusable due to a programming error.
- Breaking important system invariants but without apparent consequences.
- Buggy functionality for trusted users where a workaround exists.
- Security issues which may manifest when the system evolves.
Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.
CRITICAL SEVERITY
Resolved
Functionality limited to pool delegates, in revised code. Pool delegates are considered trusted actors.
An attacker can cause the pool to burn BPTs at highly unfavorable rates, upon handling a default.
Upon a loan default, a pool will claim the available loan funds--function claim in Pool. Anyone can call this function and it will eventually invoke handleDefault in PoolLib. This function burns BPT on Balancer using exitswapExternAmountOut and passing it infinity as the BPT number to burn (i.e., as many as needed to recover a certain amount of liquidityAsset). But Balancer is an AMM, so its pricing can be manipulated inside a single transaction via flash loan. Manipulation of the pool can yield profit for the attacker, up to the value of BPTs being burned.
HIGH SEVERITY
Resolved
Functionality limited to LPs with a large percentage of loan FDTs, in revised code. (Parameter needs to be set with care.)
Loan liquidation can be forced to suffer max slippage (10%). An attacker can make the liquidation of collateral for a defaulting loan to be done at prices 10% lower, via flash loan manipulation of the Uniswap pool.
Specifically, either anyone (after the extended grace period) or an LP provider (after the grace period but during the extended grace period) can call triggerDefault on a loan. This will eventually call triggerDefault in LoanLib, which will swap the collateral for the loan asset on Uniswap. The attacker can manipulate the Uniswap pool beforehand (e.g., via flash loan) so that it is tilted and offers 10% lower prices for the collateral asset. 10% slippage is accepted by the code (since the difference in exchange rates between Uniswap and Chainlink oracles can be significant). However, the scenario described is a predictable, repeatable attack.
Dismissed
Pool delegates are considered trusted actors in the current version of the protocol.
A pool delegate can subvert the check that a pool is well-staked (performed in Pool.finalize).
The pool delegate can take a flash loan and perform a Balancer manipulation attack, so that the check getInitialStakeRequirements in finalize, which calls the Balancer pool, succeeds. The value of the BPTs is checked against real monetary values, so the caller can make BPTs temporarily appear very expensive.
MEDIUM SEVERITY
[No medium severity issues]
LOW SEVERITY
Some ERC20 operations (transfer, transferFrom and approve) will fail, if the underlying token is not fully compliant with the ERC20 standard, as it is possible that they do not return a boolean value for the aforementioned operations to indicate the success of the call (most notable exception is USDT).
It is recommended that the OpenZeppelin SafeERC20 wrappers be used for these operations, to ensure compatibility with such tokens.
MapleGlobals.isValidCalc should be defined as a view function.
No need for public withdrawFundsOnBehalf in FDT (really, Loan).
Several contract fields are only set during the contracts’ creation. They could use the immutable modifier to reduce the gas costs of their uses.
-
PremiumCalc: premiumBips
-
LateFeeCalc: feeBips
-
FDT: fundsToken
-
MapleTreasury: mpl, fundsToken, uniswapRouter
-
MapleGlobals: mpl
-
Pool: stakingFee, delegateFee
-
Loan: apr, termDays, paymentIntervalSeconds, collateralRatio, fundingPeriodSeconds, createdAt
-
ChainlinkOracle: assetAddress
OTHER/ADVISORY ISSUES
This section details issues that are not thought to directly affect the functionality of the project, but we recommend addressing.
StakeLocker.canUnstake() relies on low-level constants of Pool. We suggest refactoring the code to hide these, in order to improve readability.
The depositDate calculation has mildly surprising consequences, especially apparent in later stages of the protocol, when lockupPeriod < penaltyDelay. The linear re-balancing of depositDate when funds are added loses information and may serve as a counter-incentive to deposits. Example:
- lockupPeriod is 0, penaltyDelay is 1yr
- user deposited 10K USDC, a year ago. User’s depositDate is now - 1yr
- user deposits another 10K USDC. User’s depositDate becomes now - 6months
- user finds themselves in need of some cash. Withdraws 5K USDC and pays penalty as if all 20K were deposited 6 months ago, although the user had 10K (i.e., more than what they are trying to withdraw) deposited a year ago.
There are a few instances of local variables that are not being explicitly initialized (assumed to be zero when first used):
- penalty in PoolLib.calcWithdrawPenalty
- Return value in BasicFDT._updateFundsTokenBalance
- losses in ExtendedFTD.recognizeLosses
- Return value in ExtendedFDT._updateLossesBalance
In LoanFactory.isValidGovernor, LoanFactory.isValidGovernorOrAdmin, and LoanFactory._whenProtocolNotPaused the revert messages refer to PoolFactory.
In PoolFactory._whenProtocolNotPaused and LoanFactory._whenProtocolNotPaused the natspec comments are wrong.
The contracts were compiled with the Solidity compiler v0.6.11 which has some known minor issues (but relatively few, compared to earlier versions). We have reviewed the issues and do not believe them to affect the contract. More specifically, at the time of writing, there are 2 known compiler bugs associated with the Solidity compiler v0.6.11:
-
Copying an empty bytes or string array from memory to storage can cause data corruption:
-
This could affect the name and symbol fields of several FDT tokens (without leading to any exploits). However the fact that these contracts are created using the appropriate factories with generated names eliminates this issue.
-
Direct assignments of storage arrays with an element size <= 16 bytes (more than one values fit in one 32 byte word) are not correctly cleared if the length of the newly assigned value is smaller than the length of the previous one. (No such array is ever stored.)
In LoanFactory.isValidGovernor, LoanFactory.isValidGovernorOrAdmin, and LoanFactory._whenProtocolNotPaused the revert messages refer to PoolFactory.
In PoolFactory._whenProtocolNotPaused and LoanFactory._whenProtocolNotPaused the natspec comments are wrong.
DISCLAIMER
The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.
ABOUT DEDAUB
Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub's auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.