Price can be easily inflated/deflated by large depositors in the Market contract

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

An attacker can manipulate/inflate market prices by donating/buying large amounts of tokens which can negatively impact subsequent transactions.

For example, an attacker who executes a large buy order can significantly increase the price of shares, causing other participants to pay an inflated price for the same shares. This will have greater effect especially if the attacker is an early buyer.

This will cause loss of trust in the protocol and potential loss of funds for subsequent users.

Proof of Concept

The problem arises due to the linear increases in share prices based on the token supply for each share in the Market contract.

After buying, the token count increases :

    function buy(uint256 _id, uint256 _amount) external {
        require(shareData[_id].creator != msg.sender, "Creator cannot buy");
        (uint256 price, uint256 fee) = getBuyPrice(_id, _amount); // Reverts for non-existing ID
        SafeERC20.safeTransferFrom(token, msg.sender, address(this), price + fee);
        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);
    }

Which in turn increase price and fee for the subsequent buys :

    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;
        }
    }

POC

This attack can be demonstrated as follows. To execute the test : forge test --mt testDonationAttack -vv

    function testDonationAttack() public {
        testCreateNewShare();

        address attacker = makeAddr("Attacker"); 
        address toto = makeAddr("Toto");
        token.mint(attacker, 10000 ether);
        token.mint(alice, 1000 ether);
        token.mint(toto, 1000 ether);

        // Alice's buy order goes through for a premium
        uint256 balanceOfAliceBefore = token.balanceOf(alice);
        vm.prank(alice);
        token.approve(address(market), 1 ether);
        vm.prank(alice);
        market.buy(1, 10);
        // console.log(token.balanceOf(alice));

        uint256 balanceOfAliceAfter = token.balanceOf(alice);
        // Costs 0.6 ether to buy 1 share
        console.logInt(int256(balanceOfAliceAfter) -int256(balanceOfAliceBefore)); // -57599999999999998

        // 
        uint256 balanceOfAttackerBefore = token.balanceOf(attacker);
        vm.prank(attacker);
        token.approve(address(market), 1000 ether);
        vm.prank(attacker);
        market.buy(1, 1000);
        console.log(token.balanceOf(attacker));

        uint256 balanceOfTotoBefore = token.balanceOf(toto);
        vm.prank(toto);
        token.approve(address(market), 11 ether);
        vm.prank(toto);
        market.buy(1, 10);
        // console.log(token.balanceOf(toto));

        uint256 balanceOfTotoAfter = token.balanceOf(toto);
        // Costs > 10 ether to buy same number of shares
        console.logInt(int256(balanceOfTotoAfter) -int256(balanceOfTotoBefore)); // -10267833333333333327
    }

Tools Used

Manual review + foundry

Recommended Mitigation Steps

Consider the following changes

• Implement a non-linear pricing curve
• Limit the size of transactions
• After creating a new share, add initial funds to make it harder for early buyers to manipulate the price. You can affect those early tokens to address(0) to enable share dilution in the same way as Uniswap.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[minhquanym]

This is design but leaving for sponsor review

[c4-sponsor]

OpenCoreCH (sponsor) disputed

[MarioPoneder]

I acknowledge the validity of those concerns. However, this is rather a design choice than a design flaw or a bug of the protocol per se, therefore QA seems appropriate.
However, the initial severity of the submission is High, therefore overinflated.

[c4-judge]

MarioPoneder marked the issue as unsatisfactory:
Overinflated severity