code-423n4 / 2024-01-curves-findings

1 stars 0 forks source link

Normal Users Potentially Cannot Sell All Curves Token #635

Open c4-bot-5 opened 10 months ago

c4-bot-5 commented 10 months ago

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L284

Vulnerability details

Impact

The issue in the sellCurvesToken() function arises from a constraint that prevents the sale of the last remaining token in the total supply of a given subject token. The intention behind this constraint is likely to maintain the existence of the token and prevent the subject from selling a token that was originally obtained for free, as per the bonding curve pricing mechanism. However, the current implementation of this constraint is overly restrictive and does not differentiate between tokens owned by the subject itself and those owned by other users.

The problematic code is as follows:

        if (supply <= amount) revert LastTokenCannotBeSold();

This line of code prevents any sale transaction that would result in the total supply of the subject token being reduced to zero. The issue is that it indiscriminately applies to all users, including those who did not receive any free tokens and bought their shares at a price set by the bonding curve.

The unintended consequence is that regular users who have purchased tokens are unable to sell their entire holdings if their holdings match the total remaining supply, even though they should logically be able to liquidate their investment.

Proof of Concept

The POC shows that normal users cannot sell all purchased tokens as expected.

Code

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.7;

import "forge-std/Test.sol";
import {CurvesERC20Factory} from "../../contracts/CurvesERC20Factory.sol";
import {FeeSplitter} from "../../contracts/FeeSplitter.sol";
import {Curves} from "../../contracts/Curves.sol";
import "solpretty/SolPrettyTools.sol";

contract CurvesBaseTest is Test, SolPrettyTools {
    CurvesERC20Factory public factory;
    FeeSplitter public feeSplitter;
    Curves public curves;

    address public FeeDestination = makeAddr("FeeDestination");
    address public ReferralFeeDestination = makeAddr("ReferralFeeDestination");
    address public Owner = makeAddr("Owner");
    address public Bob = makeAddr("Bob");
    address public Tom = makeAddr("Tom");
    address public Eva = makeAddr("Eva");

    function setUp() public virtual {
        vm.startPrank(Owner);
        factory = new CurvesERC20Factory();
        feeSplitter = new FeeSplitter();
        curves = new Curves(address(factory), address(feeSplitter));
        feeSplitter.setManager(address(curves), true);
        feeSplitter.setCurves(curves);
        curves.setManager(Owner, true);
        curves.setERC20Factory(address(factory));
        curves.setFeeRedistributor(address(feeSplitter));
        console2.log("Setup: Max Fee Percent is 0.1 ether (10%)");
        curves.setMaxFeePercent(0.1 ether);//10%
        console2.log("Setup: Protocol Fee Percent is 0.05 ether (5%)");
        curves.setProtocolFeePercent(0.05 ether, FeeDestination);
        console2.log("Setup: Subject Fee Percent is 0.02 ether (2%), Referral Fee Percent is 0.02 ether (2%), Holder Fee Percent is 0.01 ether (1%)");
        curves.setExternalFeePercent(0.02 ether, 0.02 ether, 0.01 ether);
        vm.stopPrank();

        //init funds
        deal(Bob, 1e5 ether);
        deal(Tom, 1e5 ether);

        vm.label(address(curves), "Curves");
        vm.label(address(feeSplitter), "FeeSplitter");
    }

    function userBuyCurvesTokenWithName(
        address subject,
        uint256 amount,
        string memory name,
        string memory symbol
    ) internal {
        vm.startPrank(subject);
        console2.log("%s Buys Curves %s Token with Amount %d", vm.getLabel(subject), symbol, amount);
        uint256 totalPrice = curves.getBuyPriceAfterFee(subject, amount);
        curves.buyCurvesTokenWithName{value: totalPrice}(subject, amount, name, symbol);
        vm.stopPrank();
    }

    function userBuyCurvesToken(address user, address subject, uint256 amount) internal {
        vm.startPrank(user);
        uint256 totalPrice = curves.getBuyPriceAfterFee(subject, amount);
        console2.log("%s Buys %s shares of subject %s with payment:", vm.getLabel(user), amount, vm.getLabel(subject));
        pp(totalPrice, 18, 5, " ether");
        curves.buyCurvesToken{value: totalPrice}(subject, amount);
        vm.stopPrank();
    }

    function userBuyCurvesTokenWithExtraPayment(address user, address subject, uint256 amount) internal {
        vm.startPrank(user);
        console2.log("%s Buys %s shares of subject %s with 1 ether ETH extra payment", vm.getLabel(user), amount, vm.getLabel(subject));
        uint256 totalPrice = curves.getBuyPriceAfterFee(subject, amount);
        curves.buyCurvesToken{value: totalPrice + 1 ether}(subject, amount);
        vm.stopPrank();
    }

    function userSellCurvesToken(address user, address subject, uint256 amount) internal {
        vm.startPrank(user);
        console2.log("%s Sells %d shares of subject %s", vm.getLabel(user), amount, vm.getLabel(subject));
        curves.sellCurvesToken(subject, amount);
        vm.stopPrank();
    }

    function showBalance(address _addr) internal view {
        uint256 balance = payable(_addr).balance;
        console2.log("%s's ETH balance is ", vm.getLabel(_addr));
        pp(balance, 18, 5, " ether");
    }

}

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.7;

import "./CurvesBase.t.sol";

contract CruvesTest is CurvesBaseTest {

    bytes32 public merkleRoot;
    bytes32[] public merkleProof;

    function setUp() public override {
        super.setUp();
        vm.prank(Bob);
        curves.setReferralFeeDestination(Bob, ReferralFeeDestination);
        vm.prank(Tom);
        curves.setReferralFeeDestination(Tom, ReferralFeeDestination);
    }

    function test_UserCannotSellAllTokens_buyToken_sellToken() public {
        userBuyCurvesToken(Bob, Bob, 1);
        showBalance(Bob);
        showBalance(Tom);
        userBuyCurvesToken(Tom, Bob, 100);
        userSellCurvesToken(Bob, Bob, 1);
        showBalance(Bob);
        userSellCurvesToken(Tom, Bob, 100);
        showBalance(Tom);
    }
}

Result

$ forge test --mt test_UserCannotSellAllTokens_buyToken_sellToken -vv
[⠘] Compiling...
No files changed, compilation skipped

Running 1 test for test/audit/Curves.t.sol:CruvesTest
[FAIL. Reason: LastTokenCannotBeSold()] test_UserCannotSellAllTokens_buyToken_sellToken() (gas: 596029)
Logs:
  Setup: Max Fee Percent is 0.1 ether (10%)
  Setup: Protocol Fee Percent is 0.05 ether (5%)
  Setup: Subject Fee Percent is 0.02 ether (2%), Referral Fee Percent is 0.02 ether (2%), Holder Fee Percent is 0.01 ether (1%)
  Bob Buys 1 shares of subject Bob with payment:
  0.00000  ether
  Bob's ETH balance is
  100,000.00000  ether
  Tom's ETH balance is
  100,000.00000  ether
  Tom Buys 100 shares of subject Bob with payment:
  23.26156  ether
  Bob Sells 1 shares of subject Bob
  Bob's ETH balance is
  100,000.99793  ether
  Tom Sells 100 shares of subject Bob

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 4.61ms

Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/audit/Curves.t.sol:CruvesTest
[FAIL. Reason: LastTokenCannotBeSold()] test_UserCannotSellAllTokens_buyToken_sellToken() (gas: 596029)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Foundry

Recommended Mitigation Steps

Enforce the constraint that the subject cannot sell its initial free token while allowing other users to sell all their purchased tokens. This would likely involve additional state tracking within the contract to differentiate between free and purchased tokens.

A potential implementation for the second solution could be:

        if (curvesTokenSubject == msg.sender && curvesTokenBalance[curvesTokenSubject][curvesTokenSubject] == amount) revert LastTokenCannotBeSold();

This revised condition checks if the caller (msg.sender) is the subject itself and if the transaction would result in the total supply reaching zero, which would only revert if the subject is trying to sell their last token (presumably the one they received for free).

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #22

c4-judge commented 9 months ago

alcueca changed the severity to QA (Quality Assurance)

alcueca commented 9 months ago

Very limited impact, they can sell their holdings minus one wei.

c4-judge commented 9 months ago

alcueca marked the issue as grade-a

c4-judge commented 9 months ago

This previously downgraded issue has been upgraded by alcueca

c4-judge commented 9 months ago

alcueca marked the issue as satisfactory

c4-judge commented 9 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

alcueca marked the issue as grade-b