JP_Courses - `ETH` sent to InfiltrationPeriphery.sol via unintended routes may remain trapped.

[sherlock-admin2]

JP_Courses

high

ETH sent to InfiltrationPeriphery.sol via unintended routes may remain trapped.

Summary

ETH sent to InfiltrationPeriphery.sol via unintended routes may remain trapped due to the contract's reliance on the heal() function to withdraw ETH, leaving no mechanism to recover ETH sent directly to the contract.

Vulnerability Detail

• The contract has a receive() function that allows it to accept ETH sent directly to the contract.
• The contract provides a mechanism to withdraw ETH using the heal() function via its _transferETHAndWrapIfFailWithGasLimit() method. This method uses msg.value to determine how much surplus ETH to refund to the msg.sender.
• But if anyone mistakenly or intentionally sends ETH directly to the contract without using the heal() function, there is no way to withdraw or recover this ETH because the heal() function's logic is the only way to trigger an ETH withdrawal, and it doesn't make use of address(this).balance at all.

Impact

So any ETH sent to this contract directly (accidentally or intentionally) without using the heal() function will be stuck permanently, with no way of getting it out other than maybe the deprecated selfdestruct opcode.

Code Snippet

The refund block inside the heal() function: https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/InfiltrationPeriphery.sol#L64-L69

Tool used

VSC. Manual Review

Recommendation

To address this issue, it's advisable to implement a function that allows the contract owner or an admin to withdraw unclaimed ETH sent directly to the contract. This function should have proper access control mechanisms to prevent unauthorized withdrawals during active games, ensuring that game ETH is not mistakenly withdrawn. The implementation should consider the game state and allow withdrawals only when it's safe to do so. This will provide a safeguard against permanently lost ETH in cases where it's sent directly to the contract either intentionally or accidentally/mistakenly.

Additionally, it's good practice to document these mechanisms clearly in the contract to ensure transparency and provide instructions to users regarding how to recover their ETH if it's ever sent directly to the contract.

Example implementation of such a withdraw function:

function withdrawUnstuckETH() external onlyOwner {
    require(gameNotActive(), "Withdrawal not allowed during active games");
    uint256 balance = address(this).balance;
    payable(owner()).transfer(balance);
}

[nevillehuang]

Invalid according to sherlock rules

13. Users sending ETH/native tokens accidentally just because a contract allows is not a valid medium/high.