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

Only ether received from fees can be withdrawn from `LikeRegistry.sol`.

dansp1

Summary

The contract LikeRegistry.sol has a function called LikeRegistry::likeUser which calls LikeRegistry::matchRewards. Inside this function, a fee is calculated and added to a state variable called totalFees. Only the owner of the contract can withdraw those fees, however, the LikeRegistry::withdrawFees function only transfers the amount gathered from the totalFees variable. Since the contract has a receive function, if some ether is sent to the it will permanently locked inside the contract.

Vulnerability Details

The LikeRegistry::matchRewards function where the totalFees variable is incremented.

function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0;
userBalances[to] = 0;
​
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE ) / 100;
uint256 rewards = totalRewards - matchingFees;
@> totalFees += matchingFees;
​
// Deploy a MultiSig contract for the matched users
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
​
// Send ETH to the deployed multisig wallet
(bool success, ) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

The LikeRegistry::withdrawFees function where only the value of totalFees is withdrawn from the contract.

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

Finally, the receive function:

receive() external payable {}

Impact

Any balance that does not come from the collection of fees from the LikeRegistry::matchRewards function will be locked inside the contract, since that's the only place where totalFees is incremented.

Tools Used

Manual Review

Recommendations

Create a separate function that only the owner can call that sends all the balance of the contract instead of only the value stored on the totalFees variable.

+ function withdrawBalance() external onlyOwner {
+ require(address(this).balance > 0, "No balance to withdraw");
+
+ (bool success, ) = payable(owner()).call{value: address(this).balance}("");
+ require(success, "Transfer failed");
+ }
Updates

Appeal created

n0kto Lead Judge 6 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.