Invariant is broken, one ETH deposit by user is not convertable to one lpETH, if someone manually deposits eth to contract
howlbot-integration opened this issue · 7 comments
Lines of code
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L315
Vulnerability details
Impact
Within the context of invariants, it's emphasized that when a user deposits 1 ETH, it should automatically convert to 1 lpETH. However, if an individual chooses to directly send ETH to the contract, this disrupts the conversion mechanism, diverging from the user's intended perspective.
Proof of Concept
By incorporating the provided test into the existing test file, it becomes evident that the conversion process is compromised.
function testBreakOneToOneConversion(uint256 lockAmount) public {
lockAmount = bound(lockAmount, 1, 1e36);
vm.deal(address(this), lockAmount);
prelaunchPoints.lockETH{value: lockAmount}(referral);
// Directly deposit 1 eth
address userDepositETH = makeAddr("user");
vm.deal(address(userDepositETH), 1e18);
vm.prank(userDepositETH);
(bool sent, bytes memory data) = address(prelaunchPoints).call{
value: 1e18
}("");
vm.stopPrank();
// Set Loop Contracts and Convert to lpETH
prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));
vm.warp(
prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1
);
prelaunchPoints.convertAllETH();
vm.warp(prelaunchPoints.startClaimDate() + 1);
prelaunchPoints.claim(
ETH,
100,
PrelaunchPoints.Exchange.UniswapV3,
emptydata
);
uint256 balanceLpETH = (prelaunchPoints.totalLpETH() * lockAmount) /
prelaunchPoints.totalSupply();
assertEq(prelaunchPoints.balances(address(this), ETH), 0);
assertEq(lpETH.balanceOf(address(this)), balanceLpETH);
assertEq(lpETH.balanceOf(address(this)), lockAmount);
}Tools Used
Manual review
Recommended Mitigation Steps
To resolve this issue, removing the receive function can be an effective solution. The receive function can be found in the contract code here.
Assessed type
Other
koolexcrypto changed the severity to 3 (High Risk)
koolexcrypto changed the severity to 2 (Med Risk)
koolexcrypto marked the issue as partial-75
koolexcrypto changed the severity to 3 (High Risk)
koolexcrypto marked the issue as partial-25