code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Tracking the points through the emitted event on lock is incorrect for LRT tokens #135

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L125

Vulnerability details

Impact

Users earn points based on how many lpETH they hold, since 1 lpETH equals 1 ETH, this can be calculated through the amount deposited in lock method. The issue is, when users claim their lpETH, the received ETH is likely not 1:1 plus some fees go to the 0xProtocol. This diff difference is not tracked, leading to incorrect tracking of how many points the user should earn.

Proof of Concept

The Scenario

Tools Used

Manual Review

Recommended Mitigation Steps

Track fees, let the backend handle the difference. This will be fair to other users

Assessed type

Other

0xd4n1el commented 3 months ago

Points are calculated based on the locked amount until the claiming period starts, or epoch 2. Thus users will not be their points affected if some value is lost in the swap, they will only loose the slippage they are happy to take for converting LRT to ETH.

koolexcrypto commented 3 months ago

@0xd4n1el

Thank you for your input.

Since the points are calculated by using the Locked and Withdraw events and the Claimed event is used a safeguard against tricky withdrawals, in this case a user might get X points while in fact they should get less. Wouldn't this be not in favour of the protocol and unfair to other users?

0xd4n1el commented 3 months ago

@koolexcrypto Well, it's a tradeoff we have do as a protocol. In a perfect scenario there would be no slippage and then the user would get the exact amount of points denominated in ETH. Since they accept the slippage as a risk, we compensate that by assuring a fixed amount of points unless we notice the user tricked the system. These conditions will be part of our terms of service at launch.

0xd4n1el commented 3 months ago

Just to add, the definition of "contributing" to the protocol can be debated, since TVL is still an important metric, so not only getting tokens converted into lpETH is important, and points should also reflect that.

koolexcrypto commented 3 months ago

Thank you @0xd4n1el

Understandable. To make sure we’re on the same page, assume for every 1 ETH/LRT the user receives 1 point. Let's say we have two users, one deposited 100 LRT and on claim received 99 lpETH (due to slip page), the other deposited 100 ETH and on claim received 100 lpETH. Since both deposited 100, would both get the same points (i.e. 100)?

Per my understanding for the issue above, the ratio 1:1 is broken in this case as the total-Deposited is 199 ETH but the granted points is 200

0xd4n1el commented 3 months ago

@koolexcrypto points are also time based to it is 1 point per ETH per hour and LRTs are treated special so it is 1.25 points per LRT per hour. The 1:1 ratio was only intended for ETH lockers on the amount of lpETH they would get, nothing related to points.

koolexcrypto commented 3 months ago

@0xd4n1el Thank you for the detailed info.

Apologies, I meant that the TVL would be less than the total deposits (the event tracks upon deposit) because of the slippage. Loop would simply grant LRT depositors the same LRT amount deposited anyway. Is this correct?

0xd4n1el commented 3 months ago

@koolexcrypto Yes if LRT depositors have a reasonable slippage for conversion then they get the points based on X and not on what they receive as lpETH. Also, tricking the claimings is already not an easy task and it is much easier to just exit via a normal claiming and swap yout lpETH back to ETH, so we do not see incentives to do it. We use the Claimed events as a safeguard to discourage even more this behaviour to the point of loosing points when the claimed amount is too low compared to what you had deposited.

0xd4n1el commented 3 months ago

Also to notice that lpETH holders will continue receiving points in epoch 2, and they can provide lpETH as liquidity for even more points. So there is really little incentives to try to trick the system.

0xd4n1el commented 3 months ago

And the game is already "unfair" by design for LRT providers compared to ETH ones, since LRT depositors are getting 1.25 points. This is due to partnerships with restaking protocols, and also accounts for slippage inevitable effects on the swapping part.

koolexcrypto commented 3 months ago

@0xd4n1el Thank you. Much appreciated!

Given the sponsor's input, LRT depositors will receive their due points which is even more than ETH providers by design.

c4-judge commented 3 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

koolexcrypto marked the issue as grade-c