Description: Per the README.md, a user pays 1 ETH to like a user's profile. The following require statement is at the beginning of LikeRegistry::likeUser:
require(msg.value >= 1 ether, "Must send at least 1 ETH");
The above require statement allows the user to pay more than the required 1 ETH but the excess funds are not returned to the user.
Impact: If the user pays more than 1 ETH when calling LikeRegistry::likeUser, those excess funds are not returned to the user and they are locked in the contract.
Proof of Concept:
Add the file testLikeRegistry.t.sol to the tests folder and copy the following code into the source file:
pragma solidity ^0.8.19;
import {Test, console} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
contract LikeRegistryTest is Test {
LikeRegistry likeRegistry;
SoulboundProfileNFT profileNFT;
address user1 = makeAddr("User1");
address user2 = makeAddr("User2");
address owner = address(this);
function setUp() public {
profileNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(profileNFT));
assertEq(likeRegistry.owner(), address(this));
vm.prank(user1);
profileNFT.mintProfile("User1", 25, "ipfs://profileImage");
vm.prank(user2);
profileNFT.mintProfile("User2", 30, "ipfs://profileImage");
}
function testLikeUserOverpayment() public {
vm.deal(user1, 2 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
console.log("User1 likes User2...");
vm.prank(user1);
likeRegistry.likeUser{value: 2 ether}(user2);
assertEq(user1.balance, 0 ether);
assertEq(address(likeRegistry).balance, 2 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
}
}
Run the unit test with verbosity: forge test --mt testLikeUserOverpayment -vvv
The unit test will pass and the following is logged to the screen:
Logs:
Balance of user1: 2000000000000000000
Balance of LikeRegistry: 0
User1 likes User2...
Balance of user1: 0
Balance of LikeRegistry: 2000000000000000000
Recommended Mitigation: Revert the transaction if the payment amount is not exactly 1 ETH.
function likeUser(
address liked
) external payable {
- require(msg.value >= 1 ether, "Must send at least 1 ETH");
+ require(msg.value == 1 ether, "Must send exactly 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);
}
}
Alternatively, the excess funds could be refunded back to the user.
function likeUser(
address liked
) external payable {
require(msg.value >= 1 ether, "Must send at least 1 ETH"); // @audit-XXX how do you get overpayment back?
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");
+ // Handle overpayment by refunding excess
+ uint256 excess = msg.value - 1 ether;
+ if (excess > 0) {
+ (bool success, ) = msg.sender.call{value: excess}("");
+ require(success, "Refund failed");
+ }
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);
}
}
