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

4 stars 0 forks source link

`rescueETH` function doesn't works as expected #305

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1230 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L344-L348

Vulnerability details

Impact

The rescueETH function is implemented to collect any unexpected ETH transferred to the infinityExchange.sol contract, But this function will not work as expected. The function is supposed to return the eth from the contract to the specified destination address, but it transfers only the amount of ETH send by the caller as msg.value is used here.

Proof of Concept

In InfinityExchange.sol#L1230

  /// @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');
  }

Also in InfinityStaker.sol#L344-L348

Recommended Mitigation Steps

use address(this).balance or custom amount specified by the caller for rescuing ETH.

  /// @dev used for rescuing exchange fees paid to the contract in ETH
  function rescueETH(address destination) external payable onlyOwner {
    (bool sent, ) = destination.call{value: address(this).balance}('');
    require(sent, 'failed');
  }

Or,

  /// @dev used for rescuing exchange fees paid to the contract in ETH
  function rescueETH(address destination, uint256 amount) external payable onlyOwner {
    (bool sent, ) = destination.call{value: amount}('');
    require(sent, 'failed');
  }
nneverlander commented 2 years ago

Duplicate

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