Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loanRatio lacks precision for pairs of low decimal tokens and high decimal tokens, leading to unfair conditions for either side #2114

Open
codehawks-bot opened this issue Aug 8, 2023 · 0 comments

Comments

@codehawks-bot
Copy link

loanRatio lacks precision for pairs of low decimal tokens and high decimal tokens, leading to unfair conditions for either side

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L246

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L384

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L618

Summary

Precision loss in the (debt * 10 ** 18) / collateral formula makes it inappropriate to use in certain cases, thus limiting the ways users can use the protocol efficiently.

Vulnerability Details

The protocol is intended to handle all sorts of pairs between different ERC20 tokens. It regulates the appropriate ratios between collateral and debt by using the following formula: (debt * 10 ** 18) / collateral.

The formula will be appropriate in all cases, but the case of the debt being a low-decimal high-value token like WBTC, and the collateral token being a high-decimal low-value token like PEPE for example. In such cases the above formula will have too little precision to be used properly.

Consider the following scenario:

A user wants to create a pool with WBTC as a debt token and PEPE as a collateral token. Keep in mind that PEPE is an 18-decimal token, and that WBTC is a 8-decimal one. The issue comes in because 1 WTBC is worth around 24057400000 PEPE tokens at current prices. This means that if the ratio is 1 - 24057400000 the formula will look the following way in our case:
((1 * 10 ** 8) * 10 ** 18) / (24057400000 * 10 ** 18), which will approximate to 0 at the end because it will be less than 1. The issue here is that with such an extreme case the lender will not have a viable choice of creating such a pool because the ratios with this token pair that will approximate to a whole number are a lot. So the user will always be at a loss by creating such a pool due to the rounding.

The protocol aims to handle any ERC20 tokens and pairs. It handles fair loan agreements by regulating their collateral-to-debt ratios with the formula: (debt * 10 ** 18) / collateral.

However, in specific cases like when dealing with low-decimal high-value tokens (e.g., WBTC) as debt and high-decimal low-value tokens (e.g., PEPE) as collateral, the formula's precision may become insufficient.

Consider a scenario where a user wants to create a pool with WBTC as debt (8-decimal) and PEPE as collateral (18-decimal). The issue arises because 1 WBTC is worth approximately 24,057,400,000 PEPE tokens at current prices. Consequently, the ratio 1 - 24057400000 leads to the formula: ((1 * 10 ** 8) * 10 ** 18) / (24057400000 * 10 ** 18), which approximates to 0 due to rounding. This case presents a challenge for the lender, as the pool creation becomes unviable due to the many ratios that would approximate a whole number, resulting in a loss for the user.

The above vulnerability operates under the very generous assumption that all users depositing liquidity in the protocol are very familiar with how the protocol and the ratio formula work. It may be more severe in practice due to not all users having the knowledge and understanding of an auditor or developer.

Impact

The user will not have any incentive to put money in such a pair due to unfavorable conditions. If they chose to do so they will lose their capital by someone taking a loan, that is very unfavorable to them and is actually worth more than the collateral provided.

Tools Used

Manual Review

Recommendations

Consider increasing the precision of the calculation by refactoring it in the following way:

// @audit decimalsDebt is the decimals of the debt token
(debt * 10 ** decimalsCollateral) / (collateral * 10 ** decimalsDebt)

This should fix the issue at hand and not cause overflows in other extreme cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants