Blur Exchange contest - obront'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

When an order is executed, one of the fields in both the seller and buyer's orders is amount, which dictates the number of ERC1155 tokens that should be transferred for the defined price.

However, in StandardPolicyERC1155.sol, this field is ignored, and only 1 token is transferred.

This will cause accidental loss of funds by anyone buying more than one token, and can be intentionally exploited by a seller by listing multiple ERC1155s in one listing at a high price, knowing that only one will be transferred.

Proof of Concept

A seller wants to malicious exploit this flaw in the protocol, so they list a large collection of ERC1155s for a high price. For example, let's say they list 10 NFTs for 100 WETH.

A buyer wants to execute this offer, so they submit a buying offer with 10 NFTs for 100 WETH.

The transaction executes successfully, 100 WETH is transferred from the buyer to the seller, but only 1 NFT is transferred from the seller to the buyer.

Here is a gist with a full proof of concept written as a Foundry test.

Final Results:

  Starting Seller NFT Balance:  10
  Starting Seller WETH Balance:  0
  Starting Buyer NFT Balance:  0
  Starting Buyer WETH Balance:  100000000000000000000
-------------------
  Final Seller NFT Balance:  9
  Final Seller WETH Balance:  100000000000000000000
  Final Buyer NFT Balance:  1
  Final Buyer WETH Balance:  0

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

In both functions in StandardPolicyERC1155.sol, replace the 1 with ask.amount (and add an additional check to ensure that ask.amount == bid.amount.