Submission Details
Severity:
Ineffective Fee Withdrawal Due to Zero userBalances
Summary
The function withdrawFees aims to allow the contract owner to withdraw accumulated fees. However, due to userBalances always being zero because they are not updated in the likeUser function, totalFees will also remain zero. Consequently, withdrawFees will never have any funds to withdraw as totalFees is never updated. Additionally, there's no other mechanism to withdraw funds from the contract, potentially trapping any ETH sent to it.
Vulnerability Details
Deploy the LikeRegistry contract.
Execute likes between users to generate fees, but with userBalances and totalFees not updating due to current implementation.
Attempt to withdraw fees using withdrawFees.
PoC Code:
```solidity
function testWithdrawFeesAlwaysZero() public {
address user1 = address(0x123);
address user2 = address(0x456);
vm.deal(user1, 2 ether);
vm.deal(user2, 2 ether);
vm.prank(user1);
likeRegistry.likeUser{value: 1 ether}(user2);
vm.prank(user2);
likeRegistry.likeUser{value: 1 ether}(user1);
// Check if totalFees is zero due to no balance updates
assertEq(likeRegistry.totalFees(), 0, "totalFees should be zero as userBalances are not updated");
// Attempt to withdraw fees
vm.prank(address(this)); // Assuming this contract is the owner
vm.expectRevert("No fees to withdraw");
likeRegistry.withdrawFees();
}
```
Explanation:
This PoC shows that without updating userBalances and totalFees, the withdrawFees function will always revert because there are no fees to withdraw, and any ETH sent to the contract would be stuck. Implementing the recommendations would allow for proper fee management and withdrawal.
Impact
The owner cannot access or withdraw the fees collected from the matching process, leading to funds being stuck in the contract.
Even if funds were to accumulate due to some other mechanism or if totalFees was updated, this function is the only way to remove ETH from the contract, creating a single point of failure for fund management.
Tools Used
Manual Review and foundry
Recommendations
Ensure userBalances are updated when users like others in the likeUser function:
```solidity
function likeUser(address liked) external payable {
require(msg.value == 1 ether, "Must send exactly 1 ETH");
// ... existing checks ...
userBalances[msg.sender] += msg.value; // Update this to track payments
// ... rest of the function
}```
Consider adding an emergency withdrawal function or another mechanism for asset recovery in case of unexpected scenarios:
```solidity
function emergencyWithdraw() external onlyOwner {
uint256 balance = address(this).balance;
(bool success,) = payable(owner()).call{value: balance}("");
require(success, "Emergency withdrawal failed");
}```
Updates
Appeal created
n0kto 6 months ago
Submission Judgement Published Assigned finding tags:
finding_likeUser_no_userBalances_updated
Likelihood: High, always. Impact: High, loss of funds
Rewards breakdown
nSLOC
197This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.