code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

Loss of all rewards accumulated by a minter instead of only part of it #277

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L203

Vulnerability details

Impact

Users who mint eUSD and have the minimum 5% ratio in LP to earn LBR rewards are at risk of losing all the rewards accumulated, rather than only the rewards earned while ineligible. This can happen if they drop below the minimum 5% threshold.

Proof of Concept

Lybra Finance stated in the documentation
here (archived version here ): "If the qualifier drops below the minimum 5% threshold, the user will become ineligible for subsequent esLBR emissions. Simultaneously, a bounty equal to the amount of emissions that the user has earned while ineligible will be placed. This bounty can be purchased by any user at a 50% discount in LBR."

However, in the EUSDMiningIncentives.sol file, the purchaseOtherEarnings function does not perform this check. The only check I see is for claimable earnings by calling isOtherEarningsClaimable(user). This function only checks if the ratio of LP provided by the user is below the threshold.

function isOtherEarningsClaimable(address user) public view returns (bool) {
    return (stakedLBRLpValue(user) * 10000) / stakedOf(user) < 500;
}

As a result, a user could lose all the rewards earned instead of just a small part of it.

Tools Used

Manual Review

Recommended Mitigation Steps

The logic should be completely changed to take into account the amount earned while ineligible and only allow this amount to be bought by other users.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Insufficient quality

redtigercrypto commented 1 year ago

I'm sorry that this was unsatisfactory in your eyes. But it is a real issue. The documentation clearly states that only " bounty equal to the amount of emissions that the user has earned while ineligible will be placed". However in the code, all the rewards that the user has earned will be placed as a bounty. So users will lose more rewards than they are supposed to. The code does not reflect the documentation, and there is a real risk of user losing rewards they did not think they could lose.

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-a