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

4 stars 0 forks source link

Wrong `gasCost` calculation per order #318

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/InfinityExchange.sol#L202 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L149 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L273

Vulnerability details

Impact

During a match, there's some accounting on how much gas we're spending, so that the executor can be reimbursed. The gas cost is split between multiple orders, computing the difference between the gas at the start and at the end. The gas at the start is calculated as this (inside a loop):

uint256 startGasPerOrder = gasleft() + ((startGas - gasleft()) / numMakerOrders);

The second part should mean that a "heading" gas is to be split between all numMakerOrders. However this is true only in the first iteration of the loop, indeed increases in next iterations.

The impact is that when dealing with multiple orders, the last ones pay more than the due. It can be proven by switching orders in the array.

Proof of Concept

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

Recommended Mitigation Steps

The heading part should be calculated outside the loop, or in the first iteration, and saved in a variable commonGas. Then startGasPerOrder becomes

uint256 startGasPerOrder = gasleft() + commonGas;
nneverlander commented 2 years ago

Duplicate

nneverlander commented 2 years ago

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