Platform: Code4rena
Start Date: 08/04/2021
Pot Size: $100,000 USDC
Total HM: 3
Participants: 10
Period: 14 days
Judge: Nick Johnson
Total Solo HM: 3
Id: 4
League: ETH
Rank: 3/10
Findings: 2
Award: $14,847.16
🌟 Selected for report: 2
🚀 Solo Findings: 1
🌟 Selected for report: 0xsomeone
11135.3712 USDC - $11,135.37
0xsomeone
It is possible for a user to mislead a Pool Delegate to a seemingly innocuous loan by utilizing a token with more than 18 decimals as collateral and lucrative loan terms.
The final calculation within the collateralRequiredForDrawdown of LoanLib incorrectly assumes the collateral token of a loan to be less than 18 decimals, which can not be the case as there is no sanitization conducted on the creation of a Loan via the factory. This can cause an underflow to the power of 10 which will cause the division to yield 0 and thus cause the Loan to calculate 0 as collateral required for the loan.
Line in question: https://github.com/maple-labs/maple-core/blob/031374b2609560ade825532474048eb5826dec20/contracts/library/LoanLib.sol#L235
Manual review.
We advise the same paradigm as _toWad to be applied which is secure: https://github.com/maple-labs/maple-core/blob/031374b2609560ade825532474048eb5826dec20/contracts/library/LoanLib.sol#L247
#0 - lucas-manuel
2021-04-22T13:41:58Z
We are aware that we cannot onboard liquidityAssets or collateralAssets with more that 18 decimals of precision, and will make that part of our onboarding criteria.
#1 - Arachnid
2021-04-26T22:56:49Z
This is 100% a legitimate issue that could be exploited against the contract, and using social mitigations (making this part of the onboarding strategy) when there's a technical mitigation (require()ing that the token have <= 18 decimals, or using the recommended mitigation) is insufficient and could easily lead to an exploit due to human error.
Based on the OWASP methodogology, I'm judging this as Likelihood=Low (because of the requirement to get it past human review) and Impact=High (because of the impact of the bug if it were exploited to create a 0-collateral loan and default on it), resulting in a Severity of Medium.
🌟 Selected for report: 0xsomeone
3711.7904 USDC - $3,711.79
0xsomeone
The constructor of the Maple token calculates the chainid it should assign during its execution and permanently stores it in an immutable variable. Should Ethereum fork in the feature, the chainid will change however the one used by the permits will not enabling a user to use any new permits on both chains thus breaking the token on the forked chain permanently.
Please consult EIP1344 for more details: https://eips.ethereum.org/EIPS/eip-1344#rationale
Manual review.
The mitigation action that should be applied is the calculation of the chainid dynamically on each permit invocation. As a gas optimization, the deployment pre-calculated hash for the permits can be stored to an immutable variable and a validation can occur on the permit function that ensure the current chainid is equal to the one of the cached hash and if not, to re-calculate it on the spot.
#0 - lucas-manuel
2021-04-22T13:37:02Z
Not going to implement, if Ethereum forks we will not used forked MPL