pkqs90 - `_getCollateralTokensAmountEquivalentToPrincipalTokens` uses incorrect min/max for token oracle prices, allowing attackers to use sandwich attacks to borrow a loan with very little collateral.

[sherlock-admin2]

pkqs90

high

_getCollateralTokensAmountEquivalentToPrincipalTokens uses incorrect min/max for token oracle prices, allowing attackers to use sandwich attacks to borrow a loan with very little collateral.

Summary

In LenderCommitmentGroup_Smart, when a borrower wants to borrow principal tokens, they need to provide a certain amount of collateral tokens. The value of collateral token is calculated by the TWAP and current price of the UniswapV3 pool.

The contract tries to prevent sandwich attacks (attackers manipulate price of Uniswap pool by flash loans), however since its implementation is incorrect, it fails to do so, and allowing the attackers to perform sandwich attacks.

Vulnerability Detail

In _getCollateralTokensAmountEquivalentToPrincipalTokens(), it is responsible to calculate the amount of collateral tokens required to match the amount of principal tokens being borrowed. pairPriceWithTwap is the TWAP price, and pairPriceImmediate is the current price.

Let's take principalTokenIsToken0 == true if-branch as an example, the worstCasePairPrice is the minimum of the two prices, and the result of collateralTokensAmountToMatchValue is principalTokenAmountValue * worstCasePairPrice / UNISWAP_EXPANSION_FACTOR. This is incorrect, because we should actually take the maximum of the two prices in order to make the collateral amount larger. The current implementation means that if an attacker manipulates the current UniswapV3 price pairPriceImmediate to smaller than pairPriceWithTwap, the calculation would be using it instead of the TWAP price, and result in lesser collateral amount.

This vulnerability allows the attacker to borrow a loan with very little collateral, an example attack vector is:

1. Attacker takes a large amount of flashLoan
2. Attacker performs a UniswapV3 swap, making the token1/token0 very small
3. Attacker takes a loan in LenderCommitmentGroup_Smart using very little collateral
4. Attacker pays back flashLoan

    function _getCollateralTokensAmountEquivalentToPrincipalTokens(
        uint256 principalTokenAmountValue,
        uint256 pairPriceWithTwap,
        uint256 pairPriceImmediate,
        bool principalTokenIsToken0
    ) internal pure returns (uint256 collateralTokensAmountToMatchValue) {
        if (principalTokenIsToken0) {
            //token 1 to token 0 ?
>           uint256 worstCasePairPrice = Math.min(
                pairPriceWithTwap,
                pairPriceImmediate
            );

            collateralTokensAmountToMatchValue = token1ToToken0(
                principalTokenAmountValue,
                worstCasePairPrice //if this is lower, collateral tokens amt will be higher
            );
        } else {
            //token 0 to token 1 ?
>           uint256 worstCasePairPrice = Math.max(
                pairPriceWithTwap,
                pairPriceImmediate
            );

            collateralTokensAmountToMatchValue = token0ToToken1(
                principalTokenAmountValue,
                worstCasePairPrice //if this is lower, collateral tokens amt will be higher
            );
        }
    }

    //note: the price is still expanded by UNISWAP_EXPANSION_FACTOR
    function token0ToToken1(uint256 amountToken0, uint256 priceToken1PerToken0)
        internal
        pure
        returns (uint256)
    {
        return
            MathUpgradeable.mulDiv(
                amountToken0,
                UNISWAP_EXPANSION_FACTOR,
                priceToken1PerToken0,
                MathUpgradeable.Rounding.Up
            );
    }

    //note: the price is still expanded by UNISWAP_EXPANSION_FACTOR
    function token1ToToken0(uint256 amountToken1, uint256 priceToken1PerToken0)
        internal
        pure
        returns (uint256)
    {
        return
            MathUpgradeable.mulDiv(
                amountToken1,
                priceToken1PerToken0,
                UNISWAP_EXPANSION_FACTOR,
                MathUpgradeable.Rounding.Up
            );
    }

Impact

Borrowers can easily use a sandwich attack to borrow a loan using very little collateral amount. This would cause loss of funds to the LPs of the LenderCommitmentGroup_Smart.

Code Snippet

• https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L634-L693

Tool used

Manual review

Recommendation

A few recommendations, feel free to choose from either:

1. Use the correct min/max
2. Use twap price only
3. Fetch price from oracle (e.g. Chainlink)

Duplicate of #109