DatingDapp
First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Direct Eth sent through the recieve function is not tracked and is unrecoverable

udogodwin2k22

Summary

The LikeRegistry contract contains a critical issue where ETH sent via direct transfers is neither tracked nor recoverable. This results in an ever-growing pool of stuck ETH, which is inaccessible to both users and the contract owner. The issue stems from the receive() function, which allows the contract to accept ETH but does not update any internal balance variables or provide a way to withdraw these funds.

Vulnerability details

The root cause of this issue lies in how the contract handles incoming ETH. The contract includes a receive() function, which allows it to accept ETH sent via low-level calls. However, this ETH is not assigned to any specific variable or utilized within the contract’s logic.

/// @notice Allows the contract to receive ETH
receive() external payable {}

This function ensures that any ETH sent to the contract is accepted, but it does nothing beyond that. Unlike ETH sent through likeUser(), which is properly accounted for in userBalances, ETH received via direct transfers is ignored by all internal tracking mechanisms. The withdrawFees() function, which is the only method for retrieving contract-held ETH, exclusively relies on the totalFees variable:

function withdrawFees() external onlyOwner {
require(totalFees > 0, "No fees to withdraw");
uint256 totalFeesToWithdraw = totalFees;
​
totalFees = 0;
(bool success,) = payable(owner()).call{value: totalFeesToWithdraw}("");
require(success, "Transfer failed");
}

Since totalFees is only updated within the matchRewards() function, it does not account for ETH sent directly to the contract. As a result, any ETH transferred using send(), transfer(), or call() without an associated function call remains locked within the contract indefinitely.

The test case testUntrackedETHGetsStuck() proves this issue by sending ETH directly to the contract, verifying its presence in the contract balance, and confirming that withdrawFees() is unable to retrieve it:

Proof of concept

include this in the test suite

function testUntrackedETHGetsStuck() public {
// Fund user1 before interaction
vm.deal(user1, 10 ether);
​
// User1 sends ETH directly to the contract without calling likeUser()
vm.prank(user1);
(bool success, ) = payable(address(likeRegistry)).call{value: 1 ether}("");
require(success, "Direct ETH transfer failed");
​
// Ensure the contract balance increased
uint256 contractBalance = address(likeRegistry).balance;
assertEq(contractBalance, 1 ether, "ETH should be in LikeRegistry");
​
// Verify totalFees remains 0 (ETH is not tracked)
(, bytes memory totalFeesData) = address(likeRegistry).staticcall(
abi.encodeWithSignature("totalFees()")
);
uint256 trackedFees = totalFeesData.length > 0 ? abi.decode(totalFeesData, (uint256)) : 0;
assertEq(trackedFees, 0, "Total fees should NOT include direct ETH transfers");
​
// Ensure withdrawFees() does not allow recovery
vm.prank(owner);
(bool withdrawalSuccess, ) = address(likeRegistry).call(abi.encodeWithSignature("withdrawFees()"));
assertFalse(withdrawalSuccess, "Owner should NOT be able to withdraw untracked ETH");
}

Below is the log of the output

Ran 1 test for test/LikeRegistryPoCTest.t.sol:LikeRegistryPoCTest
[PASS] testUntrackedETHGetsStuck() (gas: 28104)
Traces:
[28104] LikeRegistryPoCTest::testUntrackedETHGetsStuck()
├─ [0] VM::deal(0x0000000000000000000000000000000000000100, 10000000000000000000 [1e19])
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000100)
│ └─ ← [Return]
├─ [55] LikeRegistry::receive{value: 1000000000000000000}()
│ └─ ← [Stop]
├─ [0] VM::assertEq(1000000000000000000 [1e18], 1000000000000000000 [1e18], "ETH should be in LikeRegistry") [staticcall]
│ └─ ← [Return]
├─ [187] LikeRegistry::totalFees() [staticcall]
│ └─ ← [Revert] EvmError: Revert
├─ [0] VM::assertEq(0, 0, "Total fees should NOT include direct ETH transfers") [staticcall]
│ └─ ← [Return]
├─ [0] VM::prank(LikeRegistryPoCTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [4619] LikeRegistry::withdrawFees()
│ └─ ← [Revert] revert: No fees to withdraw
├─ [0] VM::assertFalse(false, "Owner should NOT be able to withdraw untracked ETH") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.33ms (559.80µs CPU time)
​
Ran 1 test suite in 1.20s (6.33ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

ETH was successfully sent to the contract via receive()

The log shows LikeRegistry::receive{value: 1000000000000000000}() (1 ETH was sent).

No reverts occurred, meaning the contract accepted the ETH.

The contract's balance increased

VM::assertEq(1000000000000000000 [1e18], 1000000000000000000 [1e18], "ETH should be in LikeRegistry") confirms that the contract's balance is now 1 ETH.

Calling totalFees() reverted

LikeRegistry::totalFees() [staticcall] reverted, meaning the test tried to call totalFees() but failed.

This suggests totalFees is either private or not accessible via a public getter which aligns with the contract definition.

Total fees remained 0

VM::assertEq(0, 0, "Total fees should NOT include direct ETH transfers") confirms that no ETH was tracked as fees.

withdrawFees() failed with "No fees to withdraw"

LikeRegistry::withdrawFees() reverted with revert: No fees to withdraw, proving that the untracked ETH cannot be withdrawn.

Final Assertion Passed

VM::assertFalse(false, "Owner should NOT be able to withdraw untracked ETH") confirms that the contract's owner cannot recover the ETH.

This confirms that ETH sent via direct transfers becomes trapped within the contract with no method of retrieval.

Impact

The presence of a receive() function without corresponding tracking mechanisms can lead to a serious accumulation of stranded ETH. Over time, users may inadvertently send ETH to the contract, expecting it to be used in protocol operations, only for it to remain permanently inaccessible.

Recommendation

To prevent ETH from getting stuck in the contract, by modifying the receive() function to contribute received ETH to totalFees, ensuring it can be withdrawn using withdrawFees():

receive() external payable {
totalFees += msg.value;
}
Updates

Appeal created

n0kto Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_receive_function

Not the best design, but if you send money accidentally, that's a user mistake. Informational.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.