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

Users can raise the `lpETH` tokens out of the locking process

howlbot-integration opened this issue · 5 comments

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L392
https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L315
https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/README.md?plain=1#L124

Vulnerability details

Issue Description

The protocol's main job is to lock ETH and other tokens to send lpETH tokens to users at the right time
There is an invariant that says: Deposits are active up to the lpETH contract and lpETHVault contract are set
Users deposit and lock ETH before loopActivation and their lpETH tokens correspond to the number of tokens they deposited at the right time so technically they must not be able to manipulate the lpETH tokens after loopActivation.
However, this can be done by sending ether to the contract thanks to the receive() function before the owner calls the convertAllETH function which sets the totalLpETH.
The value of totalLpETH is crucial for calculating the number of lpETH tokens users can claim
This unintentionally enables users to influence their lpETH token balance after loopActivation, violating the protocol's intended behavior and its invariant

Impact

This vulnerability allows users to manipulate their lpETH token balance after the critical loopActivation phase, potentially leading to discrepancies in the issuance of lpETH tokens. This undermines the protocol's security and reliability, impacting the expected behavior of deposit and token issuance mechanisms

Proof of Concept

Here is the scenario:

  1. Users lock ETH
  2. the owner calls the setLoopAddresses function and ends up with the deposits
  3. According to one of the main invariants, Users can't lock and deposit any tokens after setting the loopActivation
    However, Users can send ETH to the contract before the owner calls the convertAllETH which is unexpected behavior and breaks the functionality of the protocol

Tools Used

manual review

Recommended Mitigation Steps

remove the receive() function or modify it

Assessed type

Timing

koolexcrypto marked the issue as duplicate of #18

koolexcrypto changed the severity to 2 (Med Risk)

koolexcrypto changed the severity to 3 (High Risk)

koolexcrypto marked the issue as duplicate of #33

koolexcrypto marked the issue as satisfactory