Canto Application Specific Dollars and Bonding Curves for 1155s - Tricko'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-L198

Vulnerability details

Impact

The absence of slippage control in the Market::buy() or Market::sell() functions leaves users vulnerable to significant price fluctuations during periods characterized by high levels of market activity and potential sandwich attacks by MEV searchers. Possibly leading users into engaging in unfavourable buys/sells.

Proof of Concept

When examining both the buy() and sell() functions within the Market contract, it becomes apparent that the prices and fees are dependent on the current number of outstanding tokens (tokenCount). During periods of high market activity, the tokenCount can exhibit considerable fluctuations, causing prices to vary significantly. The lack of slippage control for users, aside from managing their allowances to the Market contract (which is inefficient due to the need for additional calls to set limits for each modification, adding indirections and more latency to users actions), exposes them to the risk of engaging in unfavorable buys/sells. This can also happen during calls to mintNFT/burnNFT however with diminished impact as for this cases it only affects the fees paid.

Furthermore, this vulnerability can be exploited by MEV searchers employing sandwich attacks. MEV operators can strategically sandwich the user order against their own buy()/sell() order. This manipulation can lead to potential profits for the MEV searchers and losses to users.

Consider the simplified example below for a market dealing with token X (small values will be used for ease of explanation and assume that fees are zero). A new share is created with a bounding curve with priceIncrease set to 1. After some time, 99 shares are bought. Subsequently, Alice attempts to buy 10 shares at the expected price of 1045 (calculated as the sum of 100 + 101 + 102 + ... + 109).

1. Alice sends her transacion to buy 10 shares.
2. Bob frontruns Alice and buys 100 shares for 14950 (100 + 101 + 102 + ... + 199).
3. Alice transaction is executed and she pays 2045 (200 + 201 + 202 + ... + 209).
4. Bob sells his 100 shares and receives 15950 (110 + 111 + 112 + ... + 209).

Therefore Alice pays more than she expected, while Bob ends up with a profit.

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding an additional parameters maxPrice and minPrice as exemplified below.

diff --git a/Market.sol b/Market.mod.sol
index 10bf50a..3874a18 100644
--- a/Market.sol
+++ b/Market.mod.sol
@@ -158,9 +158,12 @@ contract Market is ERC1155, Ownable2Step {
     /// @notice Buy amount of tokens for a given share ID
     /// @param _id ID of the share
     /// @param _amount Amount of shares to buy
-    function buy(uint256 _id, uint256 _amount) external {
+    function buy(uint256 _id, uint256 _amount, uint256 maxPrice) external {
         require(shareData[_id].creator != msg.sender, "Creator cannot buy");
         (uint256 price, uint256 fee) = getBuyPrice(_id, _amount); // Reverts for non-existing ID
+        if (maxPrice != 0) {
+            require( price <= maxPrice, "Price too high!");
+        }
         SafeERC20.safeTransferFrom(token, msg.sender, address(this), price + fee);
         // 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
@@ -182,8 +185,11 @@ contract Market is ERC1155, Ownable2Step {
     /// @notice Sell amount of tokens for a given share ID
     /// @param _id ID of the share
     /// @param _amount Amount of shares to sell
-    function sell(uint256 _id, uint256 _amount) external {
+    function sell(uint256 _id, uint256 _amount, uint256 minPrice) external {
         (uint256 price, uint256 fee) = getSellPrice(_id, _amount);
+        if (minPrice != 0) {
+            require( price >= minPrice, "Price too low!");
+        }
         // Split the fee among holder, creator and platform
         _splitFees(_id, fee, shareData[_id].tokensInCirculation);
         // The user also gets the rewards of his own sale (which is not the case for buys)

Assessed type

MEV