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

Lines of code

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

Vulnerability details

Impact

In the Market currently using the LinearBondingCurve, the price increases each time a token is bought. There's also a dynamic fee for each token purchase. However, this fee is usually too small, which can allow attackers to execute a sandwich attack and profit within a single block.

Here's a summary of what could potentially happen (example in PoC section):

1. The victim sends a transaction to buy a token.
2. An attacker (possibly a node operator/validator) sandwiches the victim's transaction between two of their own transactions (one to buy and one to sell). Since the price increases after the victim's purchase, the attacker can immediately take a profit. All of this can occur within a single block, so the user is unable to prevent it.

Proof of Concept

Consider the following scenario where LINEAR_INCREASE = 1e18 / 1000 = 1e16 and the current tokenCount = 15.

1. To buy token 16, one needs to pay:
   • price = 16e16
   • fee = 16e16 * (1e17 / 4) / 1e18 = 4e15 => total paid price + fee = 164e15.
2. Selling token 17, one will get back:
   • price = 17e16
   • fee = 17e17 * (1e17 / 4) / 1e18 = 4.25e15 => total received price - fee = 165.75e15.
3. If a buying transaction is sandwiched by an attacker (Attacker buys token 16, Victim buys token 17, Attacker sells token 17), the attacker can easily make a profit:
   • Profit = 165.75e15 - 164e15 = 1.75e15.

Tools Used

Manual review

Recommended Mitigation Steps

Consider modifying the fee formula to prevent attackers from profiting after deducting the fee.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

In the Market, each token's price varies (increasing or decreasing) according to the price curve. This means that users or the UI cannot accurately predict or calculate the payment amount when initiating an off-chain transaction. Consequently, a user might end up paying significantly more than expected if many other users are also buying at the same time.

Proof of Concept

Consider the following scenario:

1. Alice intends to buy a token only if the price is below 1e15. When she sees the price at 1e15, she initiates a transaction to buy.
2. However, by the time her transaction is executed, the price has surged to 1e16. But since there is no slippage protection, the transaction still goes through successfully unless she hasn't approved enough funds.

Tools Used

Manual review

Recommended Mitigation Steps

This is a common issue. If we examine any Decentralized Exchange (DEX) like Uniswap, for example, they all have a user protection parameter like minAmountOut.

Consider adding a maxAmountSpent parameter to allow users to limit the amount they are willing to spend on a buy/sell.

Assessed type

MEV

Lines of code

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

Vulnerability details

Impact

During the `buy(...) function, the buyer must pay a fee in addition to the price. This fee is distributed among all the circulating tokens.

A comment explicitly states that the caller should not claim fees from this buy. Although it is true that the caller cannot claim fees because the rewardsLastClaimedValue is calculated using the old rewards value, the fee distribution is accounted for all tokens in circulation. As a result, fees are distributed, but the caller cannot claim their portion, causing it to be permanently locked in the contract.

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

    // @audit Fees are splitted to tokensInCirculation but sender cannot claim
    _splitFees(_id, fee, shareData[_id].tokensInCirculation);
    rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

    ...
}

Proof of Concept

Consider the following scenario:

1. Alice has 3 tokens and tokensInCirculation = 10. Assume shareHolderRewardsPerTokenScaled = 1e18 and rewardsLastClaimedValue[Alice] = 1e18 (Alice is the last user to interact with the market).
2. Alice buys 2 more tokens. Since Alice was the last one to interact, there are no rewards to claim because the old reward value (1e18) is used for calculation.
3. However, the fee for buying 2 tokens is still distributed to all tokensInCirculation = 10 (which includes Alice's 3 tokens). Assume it will make shareHolderRewardsPerTokenScaled = 2e18then it will also update rewardsLastClaimedValue[Alice] = 2e18.
4. As a result, Alice cannot claim rewards for their 3 previous tokens because rewardsLastClaimedValue[Alice]is already updated, causing them to be locked in the contract forever.

Tools Used

Manual review

Recommended Mitigation Steps

If the intention is to not distribute the fee to the buyer, only call:

_splitFees(_id, fee, shareData[_id].tokensInCirculation - tokensByAddress[_id][msg.sender]);

Assessed type

Math