Blur Exchange contest - joestakey's results

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L145 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

StandardPolicyERC1155 functions canMatchMakerBid and canMatchMakerBid always return amount == 1, regardless of the function arguments.

This means that calling execute() on an agreed order of ERC1155 token with an amount > 1 will always transfer amount == 1 of the token.

Proof of Concept

• Alice generates a sell order for an ERC1155 token, of tokenID and amount == 100
• Bob generates a buy order for the same ERC1155 token, of tokenID and amount == 100.
• Alice and Bob orders match, Bob calls BlurExchange.execute(AliceOrder, BobOrder).
• Alice receives order.price, but Bob only receive 1 token of tokenId. Given that the price was agreed for 100 tokens, Bob has effectively lost 99% of order.price.

You can run the test below - add it to execution.test.ts

it('always transfers 1 token for ERC1155 orders', 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: 100, //@audit buyer order amount
    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(1); //@audit buyer receives 1 token 
    await checkBalances(
    aliceBalance,
    aliceBalanceWeth.add(priceMinusFee),
    bobBalance,
    bobBalanceWeth.sub(price),
    feeRecipientBalance,
    feeRecipientBalanceWeth.add(fee),
    );
});

Tools Used

Manual Analysis, Hardhat

Mitigation

Amend StandardPolicyERC1155.canMatchMakerAsk and StandardPolicyERC1155.canMatchMakerBid so that the amount returned is makerAmount, not 1

31:             makerAsk.price,
32:             makerAsk.tokenId,
-33:             1,
+               makerAsk.amount,
34:             AssetType.ERC1155

57:             makerBid.price,
58:             makerBid.tokenId,
-59:             1,
+               makerBid.amount,
60:             AssetType.ERC1155