No refund mechanism for over payments in LikeRegistry::likeUser can lead to loss of funds
Description
In the LikeRegistry::likeUser function there is a check requiring the msg.value is greater than or equal to >= 1 ETH, which is the cost of liking a user. However, if a user sends a value greater than 1 ETH, there is no mechanism to refund them the difference, resulting in them losing any amount over the 1 ETH
function likeUser(address liked) external payable {
@> require(msg.value >= 1 ether, "Must send at least 1 ETH");
require(!likes[msg.sender][liked], "Already liked");
require(msg.sender != liked, "Cannot like yourself");
require(profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT");
require(profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT");
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
if (likes[liked][msg.sender]) {
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
matchRewards(liked, msg.sender);
}
}
Risk
Likelihood:
Any time a user calls LikeRegistry::likeUser and sends a value greater than 1 ETH, the extra funds will be lost to the LikeRegistry contract.
Impact:
This results in a loss of funds for the users, and paying more than they should to like another user profile.
Proof of Concept
Add the following test to your testSoulboundProfileNFT.t.sol file.
function testLikeUserWithOverPayment() public {
uint256 callValue = 2 ether;
vm.deal(user, callValue);
vm.prank(user);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2);
soulboundNFT.mintProfile("Bob", 23, "ipfs://profileImage");
uint256 userBalanceBefore = user.balance;
uint256 likeRegistryBalanceBefore = address(likeRegistry).balance;
vm.prank(user);
likeRegistry.likeUser{value: callValue}(address(user2));
uint256 userBalanceAfter = user.balance;
uint256 likeRegistryBalanceAfter = address(likeRegistry).balance;
assertEq(userBalanceAfter, userBalanceBefore - callValue);
assertEq(likeRegistryBalanceAfter, likeRegistryBalanceBefore + callValue);
}
Recommended Mitigation
Add a custom error TransferFailed
Add a constant variable for LIKE_PRICE instead of using magic numbers to follow best practices
Refund any amount over the LIKE_PRICE in the LikeRegistry::likeUser function
+ error TransferFailed();
+ uint256 public constant LIKE_PRICE = 1 ether;
function likeUser(address liked) external payable {
- require(msg.value >= 1 ether, "Must send at least 1 ETH");
+ require(msg.value >= LIKE_PRICE, "Must send at least 1 ETH");
require(!likes[msg.sender][liked], "Already liked");
require(msg.sender != liked, "Cannot like yourself");
require(profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT");
require(profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT");
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
// Check if mutual like
if (likes[liked][msg.sender]) {
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
matchRewards(liked, msg.sender);
}
+ if (msg.value > LIKE_PRICE) {
+ uint256 amountToRefund = msg.value - LIKE_PRICE;
+ (bool success, ) = payable(msg.sender).call{value: amountToRefund}("");
+ if (!success) {
+ revert TransferFailed();
+ }
}
}
