With the current implementation of the withdraw() function ETH is impossible to withdraw after startClaimDate in emergency mode. It can cause an issue, as when the Loop protocol chooses to go into emergency mode, the lpETH is likely to be illiquid on all DEXes, meaning that unless the issue which caused emergency is resolved, the users essentially lost their money. Not only it heavily impacts the trust to the Loop protocol's owner who can at any time decide to start the emergency mode, but also it incentivizes users to only use LRTs as their stake that can be easily withdrawn in case of such situation.
Proof of Concept
User stakes ETH
After time passes, the owner starts the claim period
For any reason, the owner starts emergency mode
The user tries call withdraw() but it reverts due to:
if (block.timestamp >= startClaimDate) {
revert UseClaimInstead();
}
Tools Used
Manual Review
Recommended Mitigation
Instead of converting all ETH to lpETH when calling convertAllETH and then transferring it to users the contract could use lpETH.deposit() function just like for LRTs stake. This way no lpETH has to be held in the PrelaunchPoints contract making it more encouraging for the users to lock ETH rather than only LRTs and it increases the reliability of the protocol.mmended Mitigation Steps
Lines of code
https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L292
Vulnerability details
Impact
With the current implementation of the
withdraw()
functionETH
is impossible to withdraw afterstartClaimDate
in emergency mode. It can cause an issue, as when the Loop protocol chooses to go into emergency mode, thelpETH
is likely to be illiquid on all DEXes, meaning that unless the issue which caused emergency is resolved, the users essentially lost their money. Not only it heavily impacts the trust to the Loop protocol's owner who can at any time decide to start the emergency mode, but also it incentivizes users to only use LRTs as their stake that can be easily withdrawn in case of such situation.Proof of Concept
withdraw()
but it reverts due to:Tools Used
Manual Review
Recommended Mitigation
Instead of converting all ETH to
lpETH
when callingconvertAllETH
and then transferring it to users the contract could uselpETH.deposit()
function just like for LRTs stake. This way nolpETH
has to be held in thePrelaunchPoints
contract making it more encouraging for the users to lockETH
rather than only LRTs and it increases the reliability of the protocol.mmended Mitigation StepsAssessed type
Token-Transfer