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

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/f6ee8b9c7417d0f82c6f3cb01325fe4346daecbd/asD/src/asD.sol#L75-L77

Vulnerability details

Impact

Incorrect assumptions regarding the cNOTE token's exchange rate scaling cause asD.withdrawCarry() to calculate a wrongly scaled maximumWithdrawable value. This issue could potentially block owners of asD tokens from withdrawing the correct amount of rewards, potentially leaving funds trapped in the contract indefinitely.

Proof of Concept

The core of the issue lies within the withdrawCarry function of the asD contract:

// ...
uint256 maximumWithdrawable = (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) / 1e28 - totalSupply();
// ...

The function assumes that the cNOTE exchange rate is scaled by 1e28, as inferred from the comments in the code. However, according to the actual cNOTE smart contract, the exchange rate is scaled by 1e18. This discrepancy can be verified by inspecting the exchangeRateStoredInternal function and the expScale constant in the cNOTE contract source code: https://tuber.build/token/0xEe602429Ef7eCe0a13e4FfE8dBC16e101049504C?tab=read_contract (click Code -> change from Proxy to Implementation, then you can use the search)

Using the actual exchange rate at the moment (1003956909210692900 wei or 1.0039569092106929 ether), the calculation within the withdrawCarry function leads to an underflow when scaled by 1e28, thus preventing the correct computation of withdrawable funds:

(100 ether * 1003956909210692900) / 1e28 - 100 ether == -99999999989960430908 // underflow
// the example assumes that there are 100 cNOTE on the balance and 100 asD tokens in the total supply

With the correct scaling factor (1e18):

(100 ether * 1003956909210692900) / 1e18 - 100 ether == 395690921069290000 // 0.39569092106929 ether, the correct amount of funds to withdraw

Tools Used

Manual review

Recommended Mitigation Steps

Replace the erroneous 1e28 scaling factor with the correct 1e18 scaling factor in the withdrawCarry function:

        uint256 maximumWithdrawable = (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) /
-           1e28 -
+           1e18 -
            totalSupply();

By making this correction, the function will calculate the maximum amount of funds withdrawable by the owner correctly, enabling the withdrawal of all accrued rewards without unintentionally trapping funds in the contract.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/df7362f601f64ccb8afaf73ccd885109f8217498/1155tech-contracts/src/Market.sol#L147-L169 https://github.com/code-423n4/2023-11-canto/blob/df7362f601f64ccb8afaf73ccd885109f8217498/1155tech-contracts/src/Market.sol#L171-L189

Vulnerability details

Impact

Implementation of the Market.buy() and Market.sell() functions without price slippage protection can lead to users experiencing losses due to front-running (sandwich attacks). Users with large approvals for Market could unintentionally pay more or receive less due to the variable nature of the resulting share price after other users transact with higher gas prices.

Proof of Concept

The buy function as depicted in the affected code allows a user to purchase shares at the current market price. However, due to the transaction being public on the mempool, an attacker can observe the intent to buy a certain amount of shares (e.g., Bob's transaction) and can perform a transaction with a higher gas price (Alice's transaction) to execute before Bob's. This can lead to an increase in share price right before Bob's purchase is executed, causing Bob to pay a higher price for the shares.

Similarly, the sell function allows users to sell shares at the current price. An attacker can sell shares right before a seller's transaction, causing the share price to drop and thus the seller receives less.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, implement maximum accepted price for buying and minimum accepted price for selling to protect users against undesirable price slippage. This change will also discourage malicious front-running activities.

For Market.buy():

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
    require(price <= maxPrice, "Price exceeds max allowed");
    ...
}

For Market.sell():

function sell(uint256 _id, uint256 _amount, uint256 minPrice) external {
    (uint256 price, uint256 fee) = getSellPrice(_id, _amount);
    require(price >= minPrice, "Price below min allowed");
    ...
}

By implementing these changes, users will specify the maximum price they are willing to pay for each share bought and the minimum price they are willing to accept for each share sold, protecting them from potential front-running and sandwich attacks.

Assessed type

MEV