Claim and stake more than initially locked

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/1a4dc278f1b149eec7e49ffc28e02ea18c52eff3/src/PrelaunchPoints.sol#L262

Vulnerability details

Impact

User can claim and stake more tokens via claimAndStake() than he had initially locked.

Proof of Concept

Based on "Users who provide liquidity for lpETH and lock the respective LP tokens with Loop, earn a points multiplier compared to just depositing ETH/LRTs and holding lpETH." it seems staking lpETH via claimAndStake() confers an advantage over staking directly oneself (perhaps based on the emitted claimedAmount).

By transferring ETH directly to the PrelaunchPoints contract before calling claimAndStake() on non-ETH tokens this ETH will be added to the claimedAmount = address(this).balance; and then deposited in lpETH and staked. The user is thus able to stake via claimAndStake() (or the amount claimed via claim()) for funds he had not locked.

Recommended Mitigation Steps

Count the difference before and after _fillQuote().

Assessed type

Context

[c4-judge]

koolexcrypto marked the issue as duplicate of #6

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as not a duplicate

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as partial-50

[d3e4]

@koolexcrypto Why did this get partial-50? What is described here is exactly scenario 1 in #33. Other issues which also only describe the donation issue in claimAndStake() have received full duplicates, e.g. #58, #56, #55, #53.

Also, scenario 2 in #33 seems at best QA. The user is donating funds to all holders equally, so there is no incentive to do this for an attacker. And if it is possible to contribute to the total stake, without any unfair advantage, is this really an issue?

[rexjoseph]

@d3e4 I think my issue #56 being a full duplicate is correct because I also specify how the donation is then exploited. I did talk about the claimAndStake() one too in another one of my reports at #48 which is somehow getting a partial-75 even though it clears all of the impact and builds on the deposit time invariant being broken and going further into the staking rewards part.

I do agree with your take. My second issue #48 is the same as yours and only them goes further towards the rewards part for staking which I believe both should be full duplicates.

[Rhaydden]

Hi @d3e4,

I believe marking #55 as a full duplicate is appropriate because it not only addresses the donation issue in claimAndStake, but also thoroughly describes the full impact and details of how the issue could be orchestrated.

Regarding my other issue, #70, which received a partial-50 duplicate status, I agree with the judge's decision. I didn't fully explain the root cause of the issue in that report, although the impacts were explained.

Therefore, I support the judge's decision that #55 deserves the full duplicate status, contrary to your challenge.

[d3e4]

@rexjoseph, @Rhaydden, I also think that all those reports (including mine) should be full duplicates.

[koolexcrypto]

@koolexcrypto Why did this get partial-50? What is described here is exactly scenario 1 in #33. Other issues which also only describe the donation issue in claimAndStake() have received full duplicates, e.g. #58, #56, #55, #53.

Also, scenario 2 in #33 seems at best QA. The user is donating funds to all holders equally, so there is no incentive to do this for an attacker. And if it is possible to contribute to the total stake, without any unfair advantage, is this really an issue?

The quality doesn't qualify for a full credit especially the PoC. However, all dups that don't mention lock duration bypassing will get less partial credit. still re-evaluating this.

[d3e4]

The quality doesn't qualify for a full credit especially the PoC. However, all dups that don't mention lock duration bypassing will get less partial credit. still re-evaluating this.

The issue is very simple, hence a simple and succinct PoC. There is no information missing here that is present in Scenario 1 of #33 (note). I mentioned here both the direct transfer of funds and the lock bypassing, which fully constitutes Scenario 1. (I apologise if I misconstrued your comment.)

[nevillehuang]

@koolexcrypto @d3e4 imho, most issues are missing the fact that you need to bundle a transaction to avoid risking other users claiming your donated ETH. Hence, I believe partial credits is appropriate, if not, I can respect full credit as well in the discretion of the judge.

[c4-judge]

koolexcrypto marked the issue as partial-75

[koolexcrypto]

The quality doesn't qualify for a full credit especially the PoC. However, all dups that don't mention lock duration bypassing will get less partial credit. still re-evaluating this.

The issue is very simple, hence a simple and succinct PoC. There is no information missing here that is present in Scenario 1 of #33 (note). I mentioned here both the direct transfer of funds and the lock bypassing, which fully constitutes Scenario 1. (I apologise if I misconstrued your comment.)

75% Partial credit for the fact that you mentioned both. The report is not elaborative enough.

[d3e4]

@koolexcrypto @d3e4 imho, most issues are missing the fact that you need to bundle a transaction to avoid risking other users claiming your donated ETH. Hence, I believe partial credits is appropriate, if not, I can respect full credit as well in the discretion of the judge.

That would be an exploit of the exploit. It is not necessary for the #33 exploit to work, and an attacker must always consider potential counter-exploits or the effects of random user interactions etc. Also, it's fairly obvious, and very common, that exploits of this kind should be performed in a bundle. Since this is a technical detail of interest only to the attacker it should have no impact on the severity of this issue for the protocol.

[d3e4]

The quality doesn't qualify for a full credit especially the PoC. However, all dups that don't mention lock duration bypassing will get less partial credit. still re-evaluating this.

The issue is very simple, hence a simple and succinct PoC. There is no information missing here that is present in Scenario 1 of #33 (note). I mentioned here both the direct transfer of funds and the lock bypassing, which fully constitutes Scenario 1. (I apologise if I misconstrued your comment.)

75% Partial credit for the fact that you mentioned both. The report is not elaborative enough.

@koolexcrypto What elaboration is missing? If you remove Scenario 2 from #33 there are not much more words remaining than in my report. Whatever difference remains amounts to verbal fluff rather than facts. What specific relevant fact is missing? I feel like my report is being punished for the effort of making it more succinct.

[koolexcrypto]

I'm not fan of the concept of punishing and there is no such thing in this case. Please understand that, when I read a report, some are easily understandable and some are not. Quality matter.

This is an example of missing elaboration:

By transferring ETH directly to the PrelaunchPoints contract before calling claimAndStake() on non-ETH tokens this ETH will be added to the claimedAmount = address(this).balance; and then deposited in lpETH and staked. The user is thus able to stake via claimAndStake() (or the amount claimed via claim()) for funds he had not locked.

When it is too short, the burden is on the reader to track the steps and understand it, this is due to the lack of clear steps. Clear steps are quality. In some contests, some judges mark such reports immediately as insufficient proof. I didn't, because luckily it had many dups.