User called takeOrders() and takeMultipleOneOrders() functions accumulate native token fees over time. These fees end up being frozen on the contract balance. There is only one way for an owner to transfer them, a rescueETH() function, that isn’t able to access ETH balance of the contract, transferring over call’s msg.value instead.
Impact
ETH revenue of the InfinityExchange is lost as there are no means to retrieve ETH from the balance.
Proof of Concept
ETH denominated fees are left with the InfinityExchange in _transferFees’s logic:
function _transferFees(
address seller,
address buyer,
uint256 amount,
address currency
) internal {
// protocol fee
uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000;
uint256 remainingAmount = amount - protocolFee;
// ETH
if (currency == address(0)) {
// transfer amount to seller
(bool sent, ) = seller.call{value: remainingAmount}('');
require(sent, 'failed to send ether to seller');
rescueETH() method is currently the only way for an owner to retrieve ETH funds from the contract:
/// @dev used for rescuing exchange fees paid to the contract in ETH
function rescueETH(address destination) external payable onlyOwner {
(bool sent, ) = destination.call{value: msg.value}('');
require(sent, 'failed');
}
There msg.value is used as the transfer amount: only amount attached to the call is transferred over.
Lines of code
https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1228-L1232
Vulnerability details
User called takeOrders() and takeMultipleOneOrders() functions accumulate native token fees over time. These fees end up being frozen on the contract balance. There is only one way for an owner to transfer them, a rescueETH() function, that isn’t able to access ETH balance of the contract, transferring over call’s msg.value instead.
Impact
ETH revenue of the InfinityExchange is lost as there are no means to retrieve ETH from the balance.
Proof of Concept
ETH denominated fees are left with the InfinityExchange in _transferFees’s logic:
rescueETH() method is currently the only way for an owner to retrieve ETH funds from the contract:
There
msg.value
is used as the transfer amount: only amount attached to the call is transferred over.Recommended Mitigation Steps
Use the contract balance instead: