ETH funds accidentally sent to InfinityStaker cannot be retrieved

[code423n4]

Lines of code

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

Vulnerability details

rescueETH() function that aims to retrieve mistakenly sent funds cannot reach contract balance, only sending over the Ether value attached to the current call instead.

Setting the severity to medium as the case is a violation of system's auxiliary logic. Also, an ability to receive native tokens looks to be unnecessary complication of InfinityStaker, possibly introducing

Proof of Concept

rescueETH sends the msg.value of the current call:

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

  /// @dev Admin function to rescue any ETH accidentally sent to the contract
  function rescueETH(address destination) external payable onlyOwner {
    (bool sent, ) = destination.call{value: msg.value}('');
    require(sent, 'Failed to send Ether');
  }

ETH balance of a contract is updated before payable function execution:

https://stackoverflow.com/questions/69514088/solidity-subtracting-msg-value-or-addressthis-balance

This way any mistakenly sent ETH funds residing on the contract balance are inaccessible.

Recommended Mitigation Steps

If the ability to receive and then rescue ETH is needed then add contract balance to the rescueETH transfer:

  /// @dev Admin function to rescue any ETH accidentally sent to the contract
  function rescueETH(address destination) external payable onlyOwner {
-   (bool sent, ) = destination.call{value: msg.value}('');
+   (bool sent, ) = destination.call{value: address(this).balance}('');
    require(sent, 'Failed to send Ether');
  }

However, it looks like fallback(), receive() rescueETH() functions aren't needed and if there are no other considerations here the best approach is to remove all 3 functions, so no ETH can be received by the contract and there be no native funds to rescue.

[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