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

Lines of code

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

Vulnerability details

Impact

Token holder fee doesn't be split correctly when a holder of shares is going to buy shares again. So some portion of fees will be accrued in the market contract.

Proof of Concept

Let's see following scenario.

1. user1 buy 5 share.
2. user2 buy 4 share.
3. user1 buy 2 share again.
4. all of platform owner, creator, and token holders claim their rewards. At this time, market contract should only own sum of prices of shares, but below test will be failed because holder fee on step 3 was split by 9 shares(including 5 shares for user1) By design, user1 reward shouldn't include the fees of this buy, so this fees should be split by 4 shares. The balance of market will be different with sum of prices. It's the amount of (fees on step 3) * HOLDER_CUT_BPS / 10_000 * 5 / 9

    function testBuyFee() public {
        address platformOwner = address(this);
        address creator = bob;
        address user1 = vm.addr(0x3);
        address user2 = vm.addr(0x4);
        token.transfer(user1, 1e18 / 2);
        token.transfer(user2, 1e18 / 2);
        vm.prank(user1);
        token.approve(address(market), 1e18 / 2);
        vm.prank(user2);
        token.approve(address(market), 1e18 / 2);
        testCreateNewShare();
        // user1 buys 5 tokens
        (uint price1, uint fee1) = market.getBuyPrice(1, 5);
        vm.prank(user1);
        market.buy(1, 5);        

        // user2 buys 4 tokens
        (uint price2, uint fee2) = market.getBuyPrice(1, 4);
        vm.prank(user2);
        market.buy(1, 4);

        // user1 buys 2 tokens again
        (uint price3, uint fee3) = market.getBuyPrice(1, 2);
        vm.prank(user1);
        market.buy(1, 2);        

        // claim all fees
        vm.prank(platformOwner);
        market.claimPlatformFee();
        vm.prank(creator);
        market.claimCreatorFee(1);
        vm.prank(user1);
        market.claimHolderFee(1);
        vm.prank(user2);
        market.claimHolderFee(1);

        console.log("%s, %s", price1, fee1);
        console.log("%s, %s", price2, fee2);
        console.log("%s, %s", price3, fee3);

        assertEq(token.balanceOf(address(market)), price1 + price2 + price3);
    }

Tools Used

Manual review

Recommended Mitigation Steps

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L158 It can be mitigated like below

_splitFees(_id, fee, shareData[_id].tokensInCirculation - tokensByAddress[_id][msg.sender]);

Assessed type

Math