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

4 stars 0 forks source link

ETH rescue does not work #335

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/staking/InfinityStaker.sol#L345-L349 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1229-L1232

Vulnerability details

Impact

Both contracts InfinityExchange and InfinityStaker have a function rescueETH to allow an admin to rescue any ETH accidentally sent to the contracts.

However, this ETH rescue functionality does not work. The code expects ETH to be sent to this function and immediately forwards this same amount of ETH to the specified destination address leaving the remaining ETH balance of the contract untouched.

This is due to using msg.value instead of address(this).balance as the ETH amount to rescue.

Proof of Concept

InfinityStaker.rescueETH

function rescueETH(address destination) external payable onlyOwner {
    (bool sent, ) = destination.call{value: msg.value}(''); // @audit-info use  `address(this).balance` to figure out current ETH balance
    require(sent, 'Failed to send Ether');
}

InfinityExchange.rescueETH

function rescueETH(address destination) external payable onlyOwner {
    (bool sent, ) = destination.call{value: msg.value}('');  // @audit-info use  `address(this).balance` to figure out current ETH balance
    require(sent, 'failed');
}

Tools Used

Manual review

Recommended mitigation steps

Consider using the current contract ETH balance. Change the function to (also remove payable):

function rescueETH(address destination) external onlyOwner {
    (bool sent, ) = destination.call{value: address(this).balance }('');
    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