Attackers can steal fractional tokens from the protocol

[code423n4]

Lines of code

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

Vulnerability details

Impact

Attackers can steal fractional tokens from the protocol

Proof of Concept

Anyone can buy fractional tokens from pair by calling buy() function.
The protocol takes the payment in base token the amount of buyQuote(outputAmount).

Pair.sol
147:     function buy(uint256 outputAmount, uint256 maxInputAmount) public payable returns (uint256 inputAmount) {
148:         // *** Checks *** //
149:
150:         // check that correct eth input was sent - if the baseToken equals address(0) then native ETH is used
151:         require(baseToken == address(0) ? msg.value == maxInputAmount : msg.value == 0, "Invalid ether input");
152:
153:         // calculate required input amount using xyk invariant
154:         inputAmount = buyQuote(outputAmount);
155:
156:         // check that the required amount of base tokens is less than the max amount
157:         require(inputAmount <= maxInputAmount, "Slippage: amount in");//@audit should check inputAmount > 0
158:
159:         // *** Effects *** //
160:
161:         // transfer fractional tokens to sender
162:         _transferFrom(address(this), msg.sender, outputAmount);
163:
164:         // *** Interactions *** //
165:
166:         if (baseToken == address(0)) {
167:             // refund surplus eth
168:             uint256 refundAmount = maxInputAmount - inputAmount;
169:             if (refundAmount > 0) msg.sender.safeTransferETH(refundAmount);
170:         } else {
171:             // transfer base tokens in
172:             ERC20(baseToken).safeTransferFrom(msg.sender, address(this), inputAmount);
173:         }
174:
175:         emit Buy(inputAmount, outputAmount);
176:     }

The function buyQuote() calculates the necessary amount based on the baseTokenReserves() and fractionalTokenReserves().

Pair.sol
398:     function buyQuote(uint256 outputAmount) public view returns (uint256) {
399:         return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
400:     }
...
477:     function _baseTokenReserves() internal view returns (uint256) {
478:         return baseToken == address(0)
479:             ? address(this).balance - msg.value // subtract the msg.value if the base token is ETH
480:             : ERC20(baseToken).balanceOf(address(this));
481:     }

While fractional token is in 18 decimals, there are base tokens with fewer decimals. The commonly used USDC is in 6 decimals for example.
So it is possible the buyQuote() returns zero due to rounding.
For example, if base token has 6 decimals, the function will return zero for outputAmount < 10**12 (not accurate due to slippage and the subtraction in the denominator but around this).
Also note that the protocol does not require the calculated input amount to be positive. (L#157)
So attackers can get fractional tokens without any payments. (except gas)

Tools Used

Manual Review

Recommended Mitigation Steps

• Round up the calculation in buyQuote().
• Require the input amount is not zero at L#157.

[hansfriese]

buyQuote(uint256 outputAmount) takes as an input the amount of fractionalTokens in order to calculate the amount of baseTokens to receive this amount of fractionalTokens so the decimals are looking good, UniswapV2Library does the same: https://github.com/Uniswap/v2-periphery/blob/master/contracts/libraries/UniswapV2Library.sol#L57

Note that Uniswap is adding 1. It is what the mitigation is saying.

[c4-judge]

berndartmueller marked the issue as duplicate of #243

[c4-judge]

berndartmueller changed the severity to 2 (Med Risk)

[c4-judge]

berndartmueller marked the issue as satisfactory