Blur Exchange contest - MiloTruck's results

Lines of code

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

Vulnerability details

Impact

Due to the way StandardPolicyERC1155 is coded, users will only receive 1 ERC1155 token instead of the amount specified in their buy order.

Proof of Concept

In the StandardPolicyERC1155, contract, the functions canMatchMakerAsk() and canMatchMakerBid() return 1 as the amount to be transferred:

// Using canMatchMakerAsk() as an example
  24:    return (
  25:        (makerAsk.side != takerBid.side) &&
  26:        (makerAsk.paymentToken == takerBid.paymentToken) &&
  27:        (makerAsk.collection == takerBid.collection) &&
  28:        (makerAsk.tokenId == takerBid.tokenId) &&
  29:        (makerAsk.matchingPolicy == takerBid.matchingPolicy) &&
  30:        (makerAsk.price == takerBid.price),
  31:        makerAsk.price,
  32:        makerAsk.tokenId,
  33:        1, // @audit This is the amount to be transferred in an order
  34:        AssetType.ERC1155
  35:    );

Thus, if StandardPolicyERC1155 is used as the matching policy for an ERC1155 order pair, only 1 ERC1155 token will be transferred to the buyer, regardless of the amount specified in the order. Attackers could abuse this by creating sell orders with a high amount, misleading buyers into thinking they are buying more than 1 ERC1155 token.

Furthermore, this goes against what is written in the README, which states amount works for ERC1155 tokens:

The responsibility of each policy is to assert the criteria for a valid match are met and return the parameters for proper execution -

• price - matching price
• tokenId - NFT token id to transfer
• amount - (for erc1155) amount of the token to transfer
• assetType - ERC721 or ERC1155

Recommended Mitigation Steps

In StandardPolicyERC1155, check that amount is the same for both ask and bid orders, and return that instead.