Blur Exchange contest - M4TZ1P's results

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L416-L434 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L12-L62 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L128-L175

Vulnerability details

Order struct has amount as a member variable that specifies the number of tokens to be traded in the case of ERC1155. BlurExchange.execute is a function that executes a transaction with the information of the orders. During the execution of BlurExchange.execute, BlurExchange._canMatchOrders decides amount of ERC1155 to be transferred to buyer by invoking StandardPolicyERC1155.canMatchBid or StandardPolicyERC1155.canMatchMakerAsk.

There is an issue where both functions do not check whether the token amounts to be exchanged of the sell order and buy order are the same. This leads to a problem in which the orders are matched even if the amount of ERC1155 that the buyer and seller want to trade is different.

In addition, since both function always returns 1 as the amount, the amount specified in the orders will be ignored and only 1 ERC1155 token will be sent.

Impact

This may result in unintended losses to traders who wish to trade a certain number of ERC1155 tokens.

Proof of Concept

Please append the following test in line 129 of tests/execution.test.ts This test should fail to demonstrate the described scenario.

it('can exchange multiple ERC1155', async () => {
      await mockERC1155.mint(alice.address, tokenId, 100);
      sell = generateOrder(alice, {
        side: Side.Sell,
        tokenId,
        amount: 100,
        collection: mockERC1155.address,
        matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
      });
      buy = generateOrder(bob, {
        side: Side.Buy,
        tokenId,
        amount: 98,
        collection: mockERC1155.address,
        matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
      });
      sellInput = await sell.pack();
      buyInput = await buy.pack();

      await waitForTx(exchange.execute(sellInput, buyInput));

      expect(await mockERC1155.balanceOf(bob.address, tokenId)).to.be.equal(98);
      await checkBalances(
        aliceBalance,
        aliceBalanceWeth.add(priceMinusFee),
        bobBalance,
        bobBalanceWeth.sub(price),
        feeRecipientBalance,
        feeRecipientBalanceWeth.add(fee),
      );
    });

Tools Used

Sublime Text

Recommended Mitigation Steps

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L12-L62 can be changed to the following code.

    function canMatchMakerAsk(Order calldata makerAsk, Order calldata takerBid)
        external
        pure
        override
        returns (
            bool,
            uint256,
            uint256,
            uint256,
            AssetType
        )
    {
        return (
            (makerAsk.side != takerBid.side) &&
            (makerAsk.paymentToken == takerBid.paymentToken) &&
            (makerAsk.collection == takerBid.collection) &&
            (makerAsk.tokenId == takerBid.tokenId) &&
            (makerAsk.matchingPolicy == takerBid.matchingPolicy) &&
            (makerAsk.amount == takerBid.amount) &&
            (makerAsk.price == takerBid.price),
            makerAsk.price,
            makerAsk.tokenId,
            makerAsk.amount,
            AssetType.ERC1155
        );
    }

    function canMatchMakerBid(Order calldata makerBid, Order calldata takerAsk)
        external
        pure
        override
        returns (
            bool,
            uint256,
            uint256,
            uint256,
            AssetType
        )
    {
        return (
            (makerBid.side != takerAsk.side) &&
            (makerBid.paymentToken == takerAsk.paymentToken) &&
            (makerBid.collection == takerAsk.collection) &&
            (makerBid.tokenId == takerAsk.tokenId) &&
            (makerBid.matchingPolicy == takerAsk.matchingPolicy) &&
            (makerBid.amount == takerAsk.amount) &&
            (makerBid.price == takerAsk.price),
            makerBid.price,
            makerBid.tokenId,
            makerBid.amount,
            AssetType.ERC1155
        );
    }