Unchecked ETH Transfer in receive Function Allows Loss of Funds

Summary

The receive function in the SophonFarming contract allows direct deposits of ETH for the wstETH pool. However, this function does not have proper checks and error handling for ETH transfers. Specifically, it does not verify if the ETH transfer was successful, which can lead to unexpected failures or reverts. If an ETH transfer fails, the contract will not correctly handle the failure, potentially resulting in loss of funds or incorrect state updates.

Vulnerability Detail

The receive function allows deposits of ETH by calling depositEth.If the sender is weth, the function returns without doing anything. Otherwise, it calls the depositEth function with the PredefinedPool.wstETH pool. The vulnerability arises because there is no check to ensure the ETH transfer is successful. If the transfer fails for any reason (e.g., the contract is unable to accept ETH, or there is a reentrancy attack), the function will not handle it properly, potentially leading to loss of funds or other unexpected behavior.

Impact

Unchecked ETH transfers can result in loss of user funds or incorrect state updates. If the contract fails to receive ETH but proceeds with the subsequent operations, it can lead to inconsistent contract states, miscalculated rewards, or even potential exploits by malicious actors.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L92-L98

Tool used

Manual Review

Recommendation

To fix this vulnerability, add proper checks and error handling for ETH transfers. Ensure that the transfer is successful before proceeding with any state updates or further operations.

By implementing these checks, we can ensure that the contract correctly handles ETH transfers and avoids potential loss of funds due to transfer failures.

The modified receive function can be as follows:

+ error NoEthSent();
+ error EthTransferFailed();
receive() external payable {
    if (msg.sender == weth) {
        return;
    }

+    if (msg.value == 0) {
+       revert NoEthSent();
+   }
+   uint256 initialBalance = address(this).balance - msg.value;
    depositEth(0, PredefinedPool.wstETH);
+   if (address(this).balance != initialBalance + msg.value) {
+       revert EthTransferFailed();
+   }
}

sherlock-audit / 2024-05-sophon-judging

dimi6oni - Unchecked ETH Transfer in receive Function Allows Loss of Funds #225