LoopFi - Rhaydden's results

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L226-L235 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L252-L266

Vulnerability details

Impact

This could lead to the over-issuance of lpETH tokens relative to the actual amount of ETH backing them. If additional ETH is deposited into the contract (not through the standard lockETH or convertAllETH functions) and is not accounted for in totalLpETH, then when claimAndStake is executed, this extra ETH could be wrongly converted into lpETH. This misalignment could inflate the lpETH supply without proper backing which could devalue the lpETH.

Proof of Concept

The issue in the claimAndStake function, which allows users to claim their vested lpETH and immediately stake it. The function internally calls _claim, which handles the conversion of tokens to lpETH. https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L226-L235

function claimAndStake(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)
    external
    onlyAfterDate(startClaimDate)
{
    uint256 claimedAmount = _claim(_token, address(this), _percentage, _exchange, _data);
    lpETH.approve(address(lpETHVault), claimedAmount);
    lpETHVault.stake(claimedAmount, msg.sender);

    emit StakedVault(msg.sender, claimedAmount);
}

The _claim function includes logic for converting non-ETH tokens to ETH and then to lpETH: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L252-L266

if (_token != ETH) {
    uint256 userClaim = userStake * _percentage / 100;
    _validateData(_token, userClaim, _exchange, _data);
    balances[msg.sender][_token] = userStake - userClaim;

    // Swap token to ETH
    _fillQuote(IERC20(_token), userClaim, _data);

    // Convert swapped ETH to lpETH (1 to 1 conversion)
    claimedAmount = address(this).balance;
    lpETH.deposit{value: claimedAmount}(_receiver);
}

If additional ETH is present in the contract that was not converted through convertAllETH, it will be included in the address(this).balance calculation, leading to an incorrect amount of lpETH being minted based on the user's original stake.

Tools Used

Manual review

Recommended Mitigation Steps

Track any ETH deposited into the contract outside of the standard functions (lockETH, convertAllETH). Probably by modifying the fallback function to update a specific balance or state variable. Also, before converting ETH to lpETH in the _claim function, ensure that only the ETH amount corresponding to the user's stake (after token-to-ETH conversion) is used for lpETH conversion.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L257-L266 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L491-L505

Vulnerability details

Impact

When a user claims a non-ETH token, the _claim function incorrectly calculates the amount of lpETH to be minted and transferred to the user. If the contract holds any ETH balance before the token swap, this pre-existing ETH will be included in the lpETH minting, resulting in the user receiving more lpETH than they should. This could lead to inflation of the lpETH supply and loss of funds for the protocol.

Proof of Concept

The _claim function in the contract assumes that the entire ETH balance of the contract after a token-to-ETH swap is the result of that swap. This is incorrect if the contract already held some ETH before the swap. Here's the relevant part of the code: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L257-L266

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

// Convert swapped ETH to lpETH (1 to 1 conversion)
claimedAmount = address(this).balance;
lpETH.deposit{value: claimedAmount}(_receiver);

Let's consider the following scenario:

1. The contract holds a balance of 10 ETH from previous interactions.
2. A user calls the claim function to claim 1000 tokens of a non-ETH token, with a _percentage of 100.
3. The _claim function calculates userClaim as 1000 tokens and calls _fillQuote to swap these tokens for ETH.
4. The _fillQuote function performs the swap and receives 5 ETH in return.
5. After the swap, the contract's balance is now 15 ETH (10 ETH from before + 5 ETH from the swap).
6. The _claim function then calculates claimedAmount as the entire balance of the contract (15 ETH) and mints 15 lpETH to the user.

However, the user should only receive 5 lpETH, as that's the amount of ETH received from swapping their claimed tokens. The extra 10 ETH should not be included in the minting.

Tools used

Manual review

Recommended Mitigation Steps

Modify the _claim function to track the ETH balance before and after the swap, and only convert the difference to lpETH by adjusting the _fillQuote function to return the amount of ETH bought, and using this value in the _claim function:

// In _fillQuote function, return the bought ETH amount
function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns (uint256) {
    uint256 initialBalance = address(this).balance;
    require(_sellToken.approve(exchangeProxy, _amount));
    (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
    if (!success) {
        revert SwapCallFailed();
    }
    uint256 boughtETHAmount = address(this).balance - initialBalance;
    return boughtETHAmount;
}

// Adjust _claim function to use the returned value from _fillQuote
uint256 ethReceived = _fillQuote(IERC20(_token), userClaim, _data);
claimedAmount = ethReceived;
lpETH.deposit{value: claimedAmount}(_receiver);

Assessed type

ETH-Transfer