code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L272-L273

Vulnerability details

Impact

While matching the orders the gas cost is calculated so that it can be refunded back to the contract. The calculation for this gas is incorrect.

The function keeps track of the gasleft at the beginning of the loop and adds additional amount of gas for pre loop calculation inside the loop which breaks the logic.

Proof of Concept

In InfinityExchange.sol#L272-L273

The same issue repeats in matchOneToOneOrders and matchOneToManyOrders functions on the same file too.

    ...
    for (uint256 i = 0; i < numSells; ) {
      uint256 startGasPerOrder = gasleft() + ((startGas - gasleft()) / numSells);
    ...

Tools Used

Manual analysis

Recommended Mitigation Steps

The amount of gas used in the pre loop part can be calculated outside the loop

    uint256 additionalGas = (startGas - gasleft()) / numSells;
    for (uint256 i = 0; i < numSells; ) {
      uint256 startGasPerOrder = gasleft() + additionalGas;

To make it more stricter, small amount gas can also be added to account for loop iteration calculation.

nneverlander commented 2 years ago

Duplicate