search

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

4 stars 0 forks source link

Owner can grief with high gas units #338

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1260-L1263 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L739-L747

Vulnerability details

Impact

The owner of the InfinityExchange contract can frontrun sellers/buyers (e.g InfinityExchange.matchOneToOneOrders, InfinityExchange.matchOneToManyOrders functions) and set arbitrarily high gas units with updateWethTranferGas() which can either cause an underflow error or "steal" funds from the seller/buyer.

Proof of Concept

InfinityExchange.updateWethTranferGas

function updateWethTranferGas(uint32 _wethTransferGasUnits) external onlyOwner {
    WETH_TRANSFER_GAS_UNITS = _wethTransferGasUnits; // @audit-info no reasonable upper boundary
    emit NewWethTransferGasUnits(_wethTransferGasUnits);
}

WETH_TRANSFER_GAS_UNITS is passed on into the InfinityExchange._execMatchOneToOneOrders function:

InfinityExchange.sol#L739-L747

uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; // @audit-info `wethTransferGasUnits` is `WETH_TRANSFER_GAS_UNITS`
// if the execution currency is weth, we can send the protocol fee and gas cost in one transfer to save gas
// else we need to send the protocol fee separately in the execution currency
if (buy.execParams[1] == weth) {
    IERC20(weth).safeTransferFrom(buy.signer, address(this), protocolFee + gasCost); // @audit-info arbitrary high gas units can steal funds from buyer
} else {
    IERC20(buy.execParams[1]).safeTransferFrom(buy.signer, address(this), protocolFee);
    IERC20(weth).safeTransferFrom(buy.signer, address(this), gasCost); // @audit-info arbitrary high gas units can steal funds from buyer
}

Tools Used

Manual review

Recommended mitigation steps

Consider adding a reasonable max gas unit boundary for WETH_TRANSFER_GAS_UNITS in InfinityExchange.updateWethTranferGas.

HardlyDifficult commented 2 years ago

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