Using slot0 to compute position price can be easily manipulated

Summary

Using uniswap pool’s spot price by querying slot0 as a reference for the principal price is wrong, as this value can be easily manipulated.

Vulnerability Detail

The LenderCommitmentGroup_Smart contract uses the uniswap v3 pool between the principal and collateral token in order to calculate how much collateral borrowers should deposit.

This computation is given by the _calculateCollateralTokensAmountEquivalentToPrincipalTokens function:

// LenderCommitmentGroup_Smart.sol

function _calculateCollateralTokensAmountEquivalentToPrincipalTokens(
        uint256 principalTokenAmountValue
    ) internal view returns (uint256 collateralTokensAmountToMatchValue) {
        //same concept as zeroforone
        (address token0, ) = _getPoolTokens();

        bool principalTokenIsToken0 = (address(principalToken) == token0);

        uint256 pairPriceWithTwap = _getUniswapV3TokenPairPrice(twapInterval);
        uint256 pairPriceImmediate = _getUniswapV3TokenPairPrice(0);

        return
            _getCollateralTokensAmountEquivalentToPrincipalTokens(
                principalTokenAmountValue,
                pairPriceWithTwap,
                pairPriceImmediate,
                principalTokenIsToken0
            );
    }

As shown in the code snippet, two prices will be fetched from the pool:

pairPriceWithTwap: the pool’s TWAP price based on a configured twapInterval
pairPriceImmediate: this price is given by querying the pool’s slot0 function, which returns the spot price

After fetching these two values, the _getCollateralTokensAmountEquivalentToPrincipalTokens will decide which one to use based on if the principal token is token0 or token1 in the uniswap pool:

// LenderCommitmentGroup_Smart.sol
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
            );
        }
    }

The problem with this approach is that the price obtained from querying the pool’s slot0 is extremely manipulatable. slot0 returns the pool’s spot price, which is given by the current ratio of reserves in the pool. This ratio can be easily manipulated by buying/selling assets in the pool utilizing flash loans, making the spot price favorable for the attacker depending on the desired outcome.

Impact

High. A malicious user can change the pool’s spot price returned by slot0 so that the required collateral to perform a borrow is smaller than what it should actually be. That way, the attacker can perform huge borrows putting small collateral amounts.

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#L578

Tool used

Manual Review

Recommendation

Always obtain the price utilizing the TWAP instead of querying slot0.

Duplicate of #109

sherlock-audit / 2024-04-teller-finance-judging

0xadrii - Using slot0 to compute position price can be easily manipulated #235