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

[code423n4]

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.

[c4-judge]

berndartmueller marked the issue as duplicate of #442

[c4-judge]

berndartmueller changed the severity to 3 (High Risk)

[c4-judge]

berndartmueller marked the issue as satisfactory