Caviar contest - ladboy233's results

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L417 https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L77 https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L107

Vulnerability details

Impact

A malicious early user/attacker can manipulate the pricePerShare to take an unfair share of future users' deposits

The attacker can profit from future users' deposits. While the late users will lose part of their funds to the attacker.

Proof of Concept

When adding liqudity, this code is called:

// calculate the lp token shares to mint
lpTokenAmount = addQuote(baseTokenAmount, fractionalTokenAmount);

Which calls:

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);
	}
}

A malicious early user can add() with 1 wei of asset token as the first depositor of the BaseToken, and get 1 wei of shares.

Then the attacker can send 10000e18 - 1 of asset tokens and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 ( from (1 + 10000e18 - 1) / 1) .

As a result, the future user who deposits 19999e18 will only receive 1 wei (from 19999e18 * 1 / 10000e18) of shares token.

They will immediately lose 9999e18 or half of their deposits if they remove() right after the add()

Tools Used

Manual Review

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a port of the initial mints as a reserve to the DAO so that the pricePerShare can be more resistant to manipulation.

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

Vulnerability details

Impact

The code does not handle the token that are not 18 decimals well.

Proof of Concept

While the code assume thatt the baseToken has 18 decimals, there are a lot of ERC20 token that does not have 18 decimals.

For example, popular token like USDC has 6 decimals.

The code does not handle the token that are not 18 decimals well in arithemic accounting.

  function baseTokenReserves() public view returns (uint256) {
        return _baseTokenReserves();
    }

    function fractionalTokenReserves() public view returns (uint256) {
        return balanceOf[address(this)];
    }

    /// @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 code that use:

function baseTokenReserves() public view returns (uint256) {
	return _baseTokenReserves();
}

which calls:

function _baseTokenReserves() internal view returns (uint256) {
	return baseToken == address(0)
		? address(this).balance - msg.value // subtract the msg.value if the base token is ETH
		: ERC20(baseToken).balanceOf(address(this));
}

consider this case:

BUSD has 18 decimals, USDC has 6 decimals.

100 BUSD is 100000000000000000000

10 USDC is 100000000

but the Pair token (fractional token is 18 decimals). This means if the token has low decimals, the value is greated truncated. if the decimals is larger than 18, the value is large but token that has 18 decimals is rare so let us consider the token has less than 18 decimals.

given the code for example,

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);
}

let us say the fractional reserve is 10e18,

if using BUSD as baseToken, the price is 100 BUSD (100000000000000000000) * 1 ether / 10 ether.

If using USDC as baseToken, the price is greatly truncated to USDC(100000000) * 1 ether / 10 ether, which is a minial value comparing to using 18 decimals.

same truncation in low decimal token exists in all quoting accounting!

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project treat token with different deciamls with great caution.

Either check that token's decimal is 18 in the constructor of the Pair.

require(baseToken.decimals() == 18, "invalid decimals")

or scale the token decimal first before doing arithemic accounting.

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.

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#L422 https://github.com/code-423n4/2022-12-caviar/blob/0212f9dc3b6a418803dbfacda0e340e059b8aae2/src/Pair.sol#L421

Vulnerability details

Impact

Division by zero error revert transaction

Proof of Concept

There are a few division by zero error in the Pair smart contract

The first one is the function price()

function fractionalTokenReserves() public view returns (uint256) {
	return balanceOf[address(this)];
}

/// @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();
}

If the fractionalTokenReserve is 0, the price() revert in division by zero error.

While the price() is not explicitly used in the code, external contract may reply on the price() function. The function should not revert in divison by zero error.

The same divison by zero eror is in addQuote:

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);
	}
}

note the line:

uint256 fractionalTokenShare = (fractionalTokenAmount * lpTokenSupply) / fractionalTokenReserves();

and the line

uint256 baseTokenShare = (baseTokenAmount * lpTokenSupply) / baseTokenReserves();

the code precisely revert in divison by zero error.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project handle the division by zero error gracefully. Can return 0 and handle the 0 case when calling the related functoin instead of letting the transaction revert.