code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

Multiple items order can be matched to many same items order #307

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L94

Vulnerability details

Impact

Let's consider an example. Alice makes an order for an ERC1155, where she wants to buy 10 items with id=1 and 10 with id=2. This order can be matched using matchOneToManyOrders with two orders that sell both 10 items with id=1. Basically Alice gets 20 id1 instead of the expected 10 id1 + 10 id2. If id2 is a more expensive item this is definitely a problem.

Proof of Concept

I've made an hardhat test to prove the concept. Link to gist

Recommended Mitigation Steps

The functions in InfinityOrderBookComplication that check if items intersect should be reworked a little to consider possible overlapping (when dealing with multiple-to-one matching).

nneverlander commented 2 years ago

Duplicate. Fix in https://github.com/infinitydotxyz/exchange-contracts-v2/commit/bbbd362f18a2bb1992620a76e59621132b8a3d8c

nneverlander commented 2 years ago

https://github.com/code-423n4/2022-06-infinity-findings/issues/12