Infinity NFT Marketplace contest - 0xalpharush's results

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L739-L746 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L727 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1087

Vulnerability details

Impact

The function matchOneToOneOrders transfers an arbitrary amount of WETH from the user, buy.signer , in its inner call to _execMatchOneToOneOrders. The amount charged to the user is calculated dynamically based off of the gas consumption consumed during the trace. Notably, this amount is controlled by the seller since the seller's token can be malicious and purposefully consume a large amount of gas to grief the buyer. For example, when a user purchases an ERC721 token, the _transferNFTs will result in a call to an ERC721.safeTransferFrom that can exhibit any behavior such as wasting gas. This scenario is unlikely given that a buyer would have to purchase a malicious token, but the impact would be devastating as any WETH that the buyer has approved to the exchange can be lost.

This vulnerability is potentially possible in these functions as well: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L236-L242 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L787-L796

Proof of Concept

A buyer gives infinite WETH approval to the exchange contract and unknowingly purchases a malicious token from an attacker. The attacker's token wastes gas in the transfer call and causes all of the buyer's WETH to be sent to the protocol when matchOneToOneOrders is performed by the match executor.

Tools Used

Manual review

Recommended Mitigation Steps

Allow users to input a maximum fee/ gas cost they are willing to spend on each order. Pulling an arbitrary amount from a user's wallet without any restriction is a dangerous practice given that many users give large/ infinite approval to contracts.

In addition, manual gas accounting is error prone and it would make more sense to allow users to match orders themselves instead of extracting fees to compensate the matcher.