Submitted by unforgiven
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L271-L312
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L59-L116
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L245-L294
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L118-L143
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L330-L364
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L934-L951
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L145-L164
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L171-L243
Function matchOneToManyOrders() and takeOrders() and matchOrders() suppose to match sell order to buy order and should perform some checks to ensure that user specified parameters in orders which are signed are not violated when order matching happens. but There is no check in their execution flow to check that an order has different NFT token ids in each one of its collections, so even so number of tokens could be valid in order to order transfer but the number of real transferred tokens and their IDs can be different than what user specified and signed. and user funds would be lost. (because of ERC1155 there can be more than one token for a tokenId, so it would be possible to transfer it)
This is _takeOrders() and and code:
As you can see it uses canExecTakeOrder() to check that it is valid to perform matching. This is canExecTakeOrder() and areTakerNumItemsValid() and doTokenIdsIntersect() code which are used in execution flow to check orders and matching validity:
As you can see there is no logic to check that token IDs in one collection of order are different and code only checks that total number of tokens in one order matches the number of tokens specified and the ids in one order exists in other list defined. function doTokenIdsIntersect() checks to see that tokens ids in one collection can match list of specified tokens. because of this check lacking there are some scenarios that can cause fund lose for ERC1155 tokens (normal ERC721 requires more strange conditions). here is first example:
This examples shows that in verifying one to many order code should verify that one orders one  collections token ids are not duplicates. (the function doTokenIdsIntersect() doesnt check for this).
This scenario is performable to matchOneToManyOrders() and matchOrders() and but exists in their code (related check logics) too. more important things about this scenario is that it doesnt require off-chain maching engine to make mistake or malicious act, anyone can call takeOrders() if NFT tokens are ERC1155. for other NFT tokens to perform this attack it requires that seller==buyer or some other strange cases (like auto selling when receiving in one contract).
VIM
Add checks to ensure orders one collections token ids are not duplicate in doTokenIdsIntersect()
nneverlander (Infinity) confirmed and resolved:
HardlyDifficult (judge) commented:
