Blur Exchange contest - KIntern_NA's results

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L24-L35 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L50-L61

Vulnerability details

[2022-10-blur] In StandardPolicyERC1155, 2 functions canMatchMakerAsk and canMatchMakerBid always return amount = 1 with any value of amounts in buy order and sell order

tags: c4, 2022-10-blur, high

Affected code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L24-L35 https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L50-L61

Impact

2 functions canMatchMakerAsk and canMatchMakerBid in contract StandardPolicyERC1155 always return amount = 1 and they don't check matching of 2 amounts from sell order and buy order. So users might not buy or sell enough amount of nfts ERC1155 they want with the same price, and the orders about ERC1155 can be matched with any order of same tokens in the other side.

Proof of concept

Here is the script, added to function runMatchingPolicyTests, file tests/policy.test.ts

describe('StandardPolicyERC1155 issues ', () => {
      let sell2: OrderParameters;
      let buy2: OrderParameters;
      let buy5: OrderParameters;
      beforeEach(async () => {
        //alice create order sell 2 tokens ERC1155
        sell2 = generateOrder(alice, {
          side: Side.Sell,
          tokenId,
          matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
          amount: 2,
        }).parameters;

        //bob create order buy that 2 tokens ERC1155
        buy2 = generateOrder(bob, {
          side: Side.Buy,
          tokenId,
          matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
          amount: 2,
        }).parameters;

        //bob create order buy that 5 tokens ERC1155
        buy5 = generateOrder(bob, {
          side: Side.Buy,
          tokenId,
          matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
          amount: 5,
        }).parameters;
      });

      it('can match orders buy2 and sell2, return amount = 1', async () => {
        const { price, tokenId, amount } = await exchange.canMatchOrders(
          sell2,
          buy2,
        );
        expect(amount).to.equal(1);
      });

      it('can match orders buy5 and sell2, return amount = 1', async () => {
        const { price, tokenId, amount } = await exchange.canMatchOrders(
          sell2,
          buy5,
        );
        expect(amount).to.equal(1);
      });
    });

Tools Used

Manual review

Recommended Mitigation Steps

Should check matching amounts of sell order and buy order and return right amount in contract StandardPolicyERC1155. Here is example implementation for function canMatchMakerAsk, same with function canMatchMakerBid.

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.amount == takerBid.amount) && //check matching amount
            (makerAsk.matchingPolicy == takerBid.matchingPolicy) &&
            (makerAsk.price == takerBid.price) , 
            makerAsk.price,
            makerAsk.tokenId,
            makerAsk.amount, //return order amount, not 1
            AssetType.ERC1155
        );
    }