Unchecked Bonding Curve Lookups in Market validation of _id in buy() and sell() absent.

[c4-submissions]

Lines of code

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

buy() and sell() functions pass _id to to getBuyPrice()/getSellPrice() without validating it is a valid share ID. This can cause transactions to revert if invalid ID is provided.

Attackers can disrupt trades and deny service by intentionally passing invalid IDs that don't map to shares.

Proof of Concept

Likely, invalid IDs can be guessed/brute-forced buy() and sell().

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

    function sell(uint256 _id, uint256 _amount) external {
        (uint256 price, uint256 fee) = getSellPrice(_id, _amount);
        // 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)
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

        shareData[_id].tokenCount -= _amount;
        shareData[_id].tokensInCirculation -= _amount;
        tokensByAddress[_id][msg.sender] -= _amount; // Would underflow if user did not have enough tokens

        // Send the funds to the user
        SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim + price - fee);
        emit SharesSold(_id, msg.sender, _amount, price, fee);
    }

These functions pass the _id parameter to getBuyPrice() and getSellPrice() without first validating _id is a valid share ID.

This allows attackers to provide any arbitrary _id value.

If an invalid _id is provided, getBuyPrice() will look up an empty Share struct.

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

And will revert when trying to read data from the empty struct.

Market.sol#getSellPrice

    function getSellPrice(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 - _amount + 1, _amount);
    }

Imagine, an attacker could exploit this to disrupt trades because invalid IDs can be guessed/brute-forced.

1. Calling buy() or sell() with a random ID like 729:

buy(729, 1);

2. This passes invalid ID 729 to getBuyPrice()

3. getBuyPrice() reverts due to empty share lookup

4. Repeated calls with random IDs repeatedly disrupts buy/sell

5. Legitimate traders for real share IDs denied service and lose funds

Failing to validate IDs allows attackers to easily disrupt critical platform functions.

Recommended Mitigation Steps

Validate _id refers to existing share before passing to lookup functions.

require(_id > 0 && _id <= totalShares, "Invalid id");
getBuyPrice(_id, _amount);

Assessed type

Invalid Validation

[c4-pre-sort]

minhquanym marked the issue as insufficient quality report

[minhquanym]

Invalid. AI generated

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid