Blur Exchange contest - minhquanym's results

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59

Vulnerability details

Impact

In StandardPolicyERC1155 contract, the amount of ERC1155 token always return 1 instead of amount value in Order struct.

The result is when ERC1155 orders is matched, buyers will only receive 1 token even they specify amount > 1, which means loss of funds for buyer.

Proof of Concept

Script modified from execution.test.ts

it('can transfer ERC1155', async () => {
  await mockERC1155.mint(alice.address, tokenId, 10);
  sell = generateOrder(alice, {
    side: Side.Sell,
    tokenId,
    amount: 10,
    collection: mockERC1155.address,
    matchingPolicy: matchingPolicies.standardPolicyERC1155.address,
  });
  buy = generateOrder(bob, {
    side: Side.Buy,
    tokenId,
    amount: 10,
    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(10);
});

Tools Used

Manual Review

Recommended Mitigation Steps

Change the amount returned from StandardPolicyERC1155 from 1 to amount of order and also adding check to make sure amount of maker and taker’ orders are the same.