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

4 stars 0 forks source link

`rescueEth` does not transfer ether in contract #261

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1229 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L345

Vulnerability details

Impact

The InfinityStaker#rescueETH and InfinityExchange#rescueETH payable functions sends msg.value to the destination and not the ether in the contract, so the fees and accidentally transferred ether is not sent to the destination address

Proof of Concept

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

  function rescueETH(address destination) external payable onlyOwner {
    (bool sent, ) = destination.call{value: msg.value}(''); 
    require(sent, 'Failed to send Ether');
  }

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

  function rescueETH(address destination) external payable onlyOwner {  
    (bool sent, ) = destination.call{value: msg.value}('');
    require(sent, 'failed');
  }

Tools Used

Manual review

Recommended Mitigation Steps

msg.value can be replaced with address(this).balance and payable can be removed

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