Division by zero error reverts transaction

[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#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.

[outdoteth]

Not sure I agree with severity because no funds are directly at risk here. It is also questionable if returning 0 is the correct default. Technically the price is not zero. If there are no reserves in the contract then it should be assumed that the price is undefined. External contracts should have their own checks on how to handle it imo.

The second section is invalid because it is unreachable. If the lpTokenSupply > 0 then baseTokenReserves and fractionalTokenReserves will return a value greater than 0.

[c4-sponsor]

outdoteth marked the issue as disagree with severity

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-b