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

The debt in EligibilityDataProvider::requiredUsdValue() needs to be converted into USD; otherwise, it is not a correct value comparison. #156

Open
howlbot-integration bot opened this issue Aug 20, 2024 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-23 primary issue Highest quality submission among a set of duplicates 🤖_19_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/EligibilityDataProvider.sol#L187
https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/EligibilityDataProvider.sol#L197
https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/EligibilityDataProvider.sol#L274
https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/reward/EligibilityDataProvider.sol#L177

Vulnerability details

Impact

It does not align with the documentation, and the eligibility criteria for rewards are lower than what is specified by the protocol.

Proof of Concept

   function requiredUsdValue(address user) public view returns (uint256 required) {
@>>        uint256 totalNormalDebt = vaultRegistry.getUserTotalDebt(user);
@>>        required = (totalNormalDebt * requiredDepositRatio) / RATIO_DIVISOR;
        return _lockedUsdValue(required);
    }

Here, the value of totalNormalDebt should be calculated first, and then the requiredDepositRatio should be applied to that value. However, in the current implementation,

function isEligibleForRewards(address _user) public view returns (bool) {
        uint256 lockedValue = lockedUsdValue(_user);

        uint256 requiredValue = (requiredUsdValue(_user) * priceToleranceRatio) / RATIO_DIVISOR;
        return requiredValue != 0 && lockedValue >= requiredValue;
    }
function lockedUsdValue(address user) public view returns (uint256) {
        Balances memory _balances = IMultiFeeDistribution(multiFeeDistribution).getBalances(user);
        return _lockedUsdValue(_balances.locked);
    }
function _lockedUsdValue(uint256 lockedLP) internal view returns (uint256) {
        uint256 lpPrice = priceProvider.getLpTokenPriceUsd();
        return (lockedLP * lpPrice) / 10 ** 18;
    }

Based on the lockedUsdValue function and the _lockedUsdValue() function, we know that:

lockedValue = _lockedUsdValue(_balances.locked) = (_balances.locked * lpPrice) / 10**18;

uint256 requiredValue = (requiredUsdValue(_user) * priceToleranceRatio) / RATIO_DIVISOR;

This expands to:

requiredValue = _lockedUsdValue((totalNormalDebt * requiredDepositRatio) / RATIO_DIVISOR) * priceToleranceRatio / RATIO_DIVISOR;

Which further breaks down to:

requiredValue = (((totalNormalDebt * requiredDepositRatio) / RATIO_DIVISOR) * lpPrice / 10**18) * (priceToleranceRatio / RATIO_DIVISOR);

This shows the step-by-step calculation of the lockedValue and requiredValue based on the total debt, deposit ratio, and price tolerances.

Thus, the comparison lockedValue >= requiredValue becomes:

lockedLP > totalNormalDebt * (requiredDepositRatio / RATIO_DIVISOR) * (priceToleranceRatio / RATIO_DIVISOR)

Where:

•	(requiredDepositRatio / RATIO_DIVISOR) = 5%
•	(priceToleranceRatio / RATIO_DIVISOR) = 90%

This simplifies to:

lockedLP > totalNormalDebt * 5% * 90% = 4.5% * totalNormalDebt

So, the condition checks if the lockedLP is greater than 4.5% of the totalNormalDebt.

This contradicts the description in the documentation:

“Loopers maintaining a dLP value exceeding 5% of their Total Position Size qualify for LOOP token emissions to offset borrowing costs incurred from leveraging.”

In reality, because the value of one lockedLP token is lower than the value of the debt (in ETH), the number of lockedLP tokens falls far short of the actual requirement.

Tools Used

Manual Review

Recommended Mitigation Steps

In the requiredUsdValue function, the debt value is first calculated and then multiplied by the relevant ratio. In fact, Radiant Capital implements this exact approach in their code, as shown here:
https://github.com/radiant-capital/v2/blob/cd618877151896415705468f1b2a43c4b75b3c5b/contracts/radiant/eligibility/EligibilityDataProvider.sol#L186

Assessed type

Error

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_19_group AI based duplicate group recommendation bug Something isn't working primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Aug 20, 2024
howlbot-integration bot added a commit that referenced this issue Aug 20, 2024
@amarcu amarcu added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Sep 20, 2024
@c4-judge
Copy link
Contributor

koolexcrypto marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Sep 25, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Oct 2, 2024

koolexcrypto marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Oct 2, 2024
@C4-Staff C4-Staff added the M-23 label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-23 primary issue Highest quality submission among a set of duplicates 🤖_19_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants