code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L119-L121 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1228-L1232

Vulnerability details

Impact

The rescueETH function does not work as expected and the ETH balance of the contract, received from takeMultipleOneOrders and takeOrders (rare but also from fallback and receive), gets stuck on contract

Proof of Concept

With the takeMultipleOneOrders and takeOrders (also by mistake the fallback and receive) functions the contract receive ETH to pay the exchange fees, the owner withdraw these fees with the rescueETH but this function send the msg.value, what was sent in the transaction, to the destination and this it's not the balance of the contract

The fallback and receive functions worst this scenario because available the contract to receive ETH

Tools Used

Manual revision

Recommended Mitigation Steps

I recommend:

Remove the fallback, receive the contract should not receive ETH

Modify the rescueETH function:


event RescueETH(address destination, uint256 amount); // @audit optional, you can add the msg.sender but always will be the owner

/// @dev Used for rescuing exchange fees paid to the contract in ETH function rescueETH(address payable destination) external onlyOwner { require(destination != address(0), "The destination can't be the address 0"); // @audit optional uint256 balance = address(this).balance; (bool sent, ) = destination.call{value: balance}(''); require(sent, 'Failed to send Ether'); emit RescueETH(destination, balance); // @audit optional }



> Note: A contract can use selfdestruct to send ETH to the contract, but why would someone do this?

nneverlander commented 2 years ago

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

nneverlander commented 2 years ago

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

HardlyDifficult commented 2 years ago

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

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