Protocol can steal WETH founds

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L123-L294

Vulnerability details

Impact

The protocol can steal WETH founds with the refunds gas cost mechanism in the functions matchOneToOneOrders, matchOneToManyOrders and matchOrders This functions can call only by the MATCH_EXECUTOR but we don't know what is this contract/address according the sponsor in discord this code is not public

Proof of Concept

1) Alice approve WETH to use the exchange, generally this approve it's for max amount to save gas for the future approbes 2) Alice maker an order to sell/buy 3) When the MATCH_EXECUTOR send matchOneToManyOrders, with the Alice signature, it send the transaction with a high amount of gas 4) In the L231 the gasCost will be high 5) In the L237 or L240 the protocol take the gasCost from Alice to the exchange 6) The remaining gas of the transaction will return to the sender of the transaction

Recommended Mitigation Steps

The best approach its remove the refunds gas cost mechanism, and use other mechanism for charge fees in a easy way, like a fixed amount of X token Other option it's add another mechanism that gives to the signer of the order the max amount of fee in WETH who is willing to pay In OrderTypes library:

...
  struct MakerOrder {
    ///@dev is order sell or buy
    bool isSellOrder;
    ///@dev signer of the order (maker address)
    address signer;
+   ///@dev Maximum WETH to pay gas cost mechanism
+   uint256 maxWETHGasCost;
    ///@dev Constraints array contains the order constraints. Total constraints: 6. In order:
    // numItems - min (for buy orders) / max (for sell orders) number of items in the order
    // start price in wei
    // end price in wei
    // start time in block.timestamp
    // end time in block.timestamp
    // nonce of the order
    uint256[] constraints;
    ///@dev nfts array contains order items where each item is a collection and its tokenIds
    OrderItem[] nfts;
    ///@dev address of complication for trade execution (e.g. InfinityOrderBookComplication), address of the currency (e.g., WETH)
    address[] execParams;
    ///@dev additional parameters like traits for trait orders, private sale buyer for OTC ordes etc
    bytes extraParams;
    ///@dev the order signature uint8 v: parameter (27 or 28), bytes32 r, bytes32 s
    bytes sig;
  }
...

In InfinityExchange.sol add this lines:

In L231:
  uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice;
+ require(gasCost <= makerOrder.maxWETHGasCost, "The gasCost it's to high");
In L739:
  uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice;
+ require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high");
In L787: 
  uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice;
+ require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high");
In L893:
  uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice;
+ require(gasCost <= buy.maxWETHGasCost, "The gasCost it's to high");

[KenzoAgada]

The calculation in L231 is:

      uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice;

As far as I see, this will only transfer from Alice the actual gas cost of the transaction. Not an arbitrary amount. So the issue seems invalid.

[nneverlander]

Duplicate

[HardlyDifficult]

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