Canto Application Specific Dollars and Bonding Curves for 1155s - lanrebayode77's results

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/b78bfdbf329ba9055ba24bd710c7e1c60251039a/1155tech-contracts/src/Market.sol#L159

Vulnerability details

Impact

Unclaimable fees will remain stuck in the market contract.

Fees are split among platform owner, share creator and share holders. See the sharing formula below.

uint256 public constant NFT_FEE_BPS = 1_000; // 10%
    uint256 public constant HOLDER_CUT_BPS = 3_300; // 33%
    uint256 public constant CREATOR_CUT_BPS = 3_300; // 33%
    // Platform cut: 100% - HOLDER_CUT_BPS - CREATOR_CUT_BPS

To mitigate the issue of fees becoming stuck when there are no tokens in circulation, holder shares are added to platform fees. However, when a user makes consecutive purchase. Although the check for tokens in circulation is greater than zero, the fees may still be unclaimable. This occurs because, after the fee split, the buyer is prohibited from claiming a portion of the fees from that specific purchase by setting rewardsLastClaimedValue[_id][msg.sender] after the fee split.

rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

Proof of Concept

Instance

1. Alice calls buy(), she transfers price and fee to the market.

// The reward calculation has to use the old rewards value (pre fee-split) to not include the fees of this buy
        // The rewardsLastClaimedValue then needs to be updated with the new value such that the user cannot claim fees of this buy
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        // Split the fee among holder, creator and platform
        _splitFees(_id, fee, shareData[_id].tokensInCirculation);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

        shareData[_id].tokenCount += _amount;
        shareData[_id].tokensInCirculation += _amount;
        tokensByAddress[_id][msg.sender] += _amount;

        if (rewardsSinceLastClaim > 0) {
            SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);

To prevent her from sharing from her purchase, rewardsSinceLastClaim is obtained using the previous state, as stated in the natspec. _splitFees() is then called to set fee split, for the first purchase shareHolderRewardsPerTokenScaled is zero, since the token in circulation is zero, shareholders split is added to platform fees. 2. Alice decides to make another purchase and calls buy(), this time

 if (_tokenCount > 0) {
            shareData[_id].shareHolderRewardsPerTokenScaled += (shareHolderFee * 1e18) / _tokenCount;

but Alice rewardsLastClaimedValue[_id][msg.sender] is set to shareData[_id].shareHolderRewardsPerTokenScaled after the fee split, so she cannot get anything from calling claimHoldersFees, the problem here is, she is the only holder at the moment, so no holder can claim the fees.

3. If another user then calls buy(), Alice can then claim the holder share of the other user fees, but there is still holder fees left unclaimed as the third user cannot also claim fees from previous purchase.

Let's obtain price and fees by pasting the code below in LinearBondingCurve.t.sol

    function testgetPriceAndFee() public {
        (uint256 price1, uint256 fee1) = bondingCurve.getPriceAndFee(1, 1);
        console.log("Price1:", price1, "Fee1:", fee1);

        (uint256 price2, uint256 fee2) = bondingCurve.getPriceAndFee(2, 1);
        console.log("Price2:", price2, "Fee2:", fee2);

        (uint256 price3, uint256 fee3) = bondingCurve.getPriceAndFee(3, 1);
        console.log("Price3:", price3, "Fee3:", fee3);

        (uint256 price4, uint256 fee4) = bondingCurve.getPriceAndFee(4, 1);
        console.log("Price4:", price4, "Fee4:", fee4);

        (uint256 price5, uint256 fee5) = bondingCurve.getPriceAndFee(5, 1);
        console.log("Price5:", price5, "Fee5:", fee5);

        //LOGS
        // Price1: 1000000000000000 Fee1: 100000000000000
        // Price2: 2000000000000000 Fee2: 200000000000000
        // Price3: 3000000000000000 Fee3: 300000000000000
        // Price4: 4000000000000000 Fee4: 200000000000000
        // Price5: 5000000000000000 Fee5: 250000000000000
    }

paste the PoC below into Market.t.sol

  function test_PoC_Unclaimable_Fees() public {
        testCreateNewShare();
        address canto = address(3);
        token.transfer(alice, 1e17);
        token.transfer(canto, 1e17);
        console.log(
            "================================================================================="
        );
        console.log("Alice Initial Balance", token.balanceOf(alice));
        console.log("Market Initial Balance", token.balanceOf(address(market)));
        vm.prank(alice);
        token.approve(address(market), 1e18);
        vm.prank(canto);
        token.approve(address(market), 1e18);

        //Alice first purchase
        vm.prank(alice);
        market.buy(1, 1);
        assertEq(
            token.balanceOf(address(market)),
            LINEAR_INCREASE + LINEAR_INCREASE / 10
        );

        //shareHolderRewardsPerTokenScaled after first buy should be zero
        (, , uint256 share, , , , ) = market.shareData(1);
        assertEq(share, 0);
        console.log("share_Holder_Rewards_PerToken_Scaled1:", share);

        //Second Buy by Alice
        uint256 aliceBalBeforeSecondBuy = token.balanceOf(alice);
        vm.prank(alice);
        market.buy(1, 1);
        uint256 aliceBalAfterSecondBuy = token.balanceOf(alice);
        assertEq(
            token.balanceOf(alice),
            aliceBalBeforeSecondBuy - 2200000000000000
        ); //Check fee deduction
        //shareHolderRewardsPerTokenScaled after second buy
        (, , uint256 share2, , , , ) = market.shareData(1);
        console.log("share_Holder_Rewards_PerToken_Scaled2:", share2);

        //First Claim by Alice should be zero because she made the second purchase and she is not eligible for any holder share
        vm.prank(alice);
        market.claimHolderFee(1);
        // checks after claim
        (, , uint256 share0, , , , ) = market.shareData(1);
        //console.log("shareHolderRewardsPerTokenScaled0:", share0);
        assertEq(token.balanceOf(alice), aliceBalAfterSecondBuy); // balance remains the same as no holder share is claimable

        //Third buy by another user Canto
        vm.prank(canto);
        market.buy(1, 1);
        // checks for holder shares
        (, , uint256 share3, , , , ) = market.shareData(1);
        console.log("share_Holder_Rewards_PerToken_Scaled3:", share3);

        //Check if the market now contains all deposotes from price and fee
        uint256 allPrice = 1000000000000000 +
            2000000000000000 +
            3000000000000000;
        uint256 allFees = 100000000000000 + 200000000000000 + 300000000000000;
        assertEq(token.balanceOf(address(market)), allPrice + allFees);
        console.log(
            "Market Token Balance after 3 Purchases:",
            allPrice + allFees
        );

        //Alice now tries to claim(holder share, she has two tokens)
        vm.prank(alice);
        market.claimHolderFee(1);
        //amount = ((shareData[_id].shareHolderRewardsPerTokenScaled - lastClaimedValue) * tokensByAddress[_id][msg.sender]) / 1e18;
        //remeber alice has bought two tokens
        assertEq(
            token.balanceOf(alice),
            (((share3 - share2) * 2) / 1e18) + aliceBalAfterSecondBuy
        );
        console.log(
            "Amount claimed by Alice:",
            token.balanceOf(alice) - aliceBalAfterSecondBuy
        );

        //Creator(Bob) claims creator fee
        uint256 creatorInitialBalance = token.balanceOf(bob);
        (, , , uint256 creatorFee, , , ) = market.shareData(1);
        vm.prank(bob);
        market.claimCreatorFee(1);
        assertEq(token.balanceOf(bob), creatorInitialBalance + creatorFee);

        //Platform Owner Claims Fee
        uint256 platformOwnerInitialBalance = token.balanceOf(address(this));
        uint256 platformFee = market.platformPool();
        market.claimPlatformFee();
        assertEq(
            token.balanceOf(address(this)),
            platformOwnerInitialBalance + platformFee
        );

        //After all parties have claimed their fee, all that should remain is total price paid for all 3 tokens
        //However, there will still be unclaimed fees due to the fact that first buyer bought twice consecutively and the holder share was not claimable by anyone
        assertGt(token.balanceOf(address(market)) - allPrice, 0);
        assertEq(token.balanceOf(address(market)), allPrice + share2 / 1e18);
        console.log(
            "Unclaimable Fees:",
            token.balanceOf(address(market)) - allPrice
        );
        console.log(
            "================================================================================="
        );
    }

LOGS

Logs:
  =================================================================================
  Alice Initial Balance: 100000000000000000
  Market Initial Balance: 0
  share_Holder_Rewards_PerToken_Scaled1: 0
  share_Holder_Rewards_PerToken_Scaled2: 66000000000000000000000000000000
  share_Holder_Rewards_PerToken_Scaled3: 115500000000000000000000000000000
  Market Token Balance after 3 Purchases: 6600000000000000
  Amount claimed by Alice: 99000000000000
  Unclaimable Fees: 66000000000000
  =================================================================================

Tools Used

Manual review and Foundry.

Recommended Mitigation Steps

Add a method that is callable only by owner to sweep any unclaimed token in the Market contract.

Assessed type

ERC20