Funds(ETH) permanent lock on `InfinityStaker.sol`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L344-L348 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L54-L57

Vulnerability details

Impact

The rescueETH function does not work as expected and if the contract receives ETH, it gets stuck in the contract

Proof of Concept

If an address(wallet or contract) send ETH to the InfinityStaker.sol contract, the owner(admin) can't rescue the ETH because the rescueETH 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

Recommended Mitigation Steps

I recommend remove the fallback, receive and rescueETH functions, there are no function payable(except rescueETH) in the contract and the contract should not receive ETH

However, if for some strange reason you want to rescue ETH(see the note), I recommend fixing the rescueETH function Also, optional, create another function to rescue ERC20 tokens

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

/// @dev Admin function to rescue any ETH accidentally sent to the contract(only possible if a contract use selfdestruct) 
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
}

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

/// @dev Admin function to rescue any ERC20 accidentally sent to the contract
function rescueTokens(IERC20 currency, address destination, uint256 amount) external onlyOwner {
    require(destination != address(0), "The destination can't be the address 0"); // @audit optional
    require(currency != IERC20(INFINITY_TOKEN), "The staked tokens can't be rescue");
    currency.safeTransfer(destination, amount);
    emit RescueTokens(currency, destination, amount); // @audit optional    
}

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

[nneverlander]

Duplicate

[nneverlander]

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

[HardlyDifficult]

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