DatingDapp

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

[L-01] ETH May Become Permanently Locked in LikeRegistry

Description:

The LikeRegistry contract may lock ETH permanently if funds are sent to it outside of LikeRegistry::likeUser(), as the contract only tracks ETH collected through this function.

The issue arises because LikeRegistry::withdrawFees() attempts to transfer only the amount tracked in totalFees, rather than the actual ETH balance of the contract. Any ETH received outside of the expected flow will not be withdrawable, causing it to be locked.

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");
}

Impact:

Low – No user funds are lost, and protocol functionality remains unaffected. However, if ETH is mistakenly sent to the contract in an unexpected way, it will become permanently locked since withdrawFees() does not allow the owner to withdraw funds beyond the tracked totalFees.

Proof of Concept:

In the test below, a user sends 1 ETH directly to the contract. Since totalFees is not updated, the owner is unable to withdraw this ETH.

function testSendEthAndWithdraw() public {
LikeRegistry likeRegistry;
address user = address(0x123);
address owner = makeAddr("owner"); // Test contract acts as the owner
uint256 public constant STARTING_USER_BALANCE = 10e18;
likeRegistry.transferOwnership(owner);
vm.deal(user, STARTING_USER_BALANCE);
vm.startPrank(user);
(bool success, ) = payable(address(likeRegistry)).call{value: 1 ether}(
""
);
require(success, "Transfer failed");
assertEq(address(likeRegistry).balance, 1 ether);
assertEq(likeRegistry.totalFees(), 0);
vm.stopPrank();
vm.startPrank(owner);
//No fees to withdraw
likeRegistry.withdrawFees();
}

Recommended Mitigation:

Modify withdrawFees() to withdraw the contract’s actual balance rather than the tracked totalFees:

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

This ensures that any ETH held by the contract, regardless of how it was received, can be withdrawn, preventing funds from becoming permanently locked.

Updates

Appeal created

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

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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