DatingDapp

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

Mishandling of overpayment while executing `LikeRegistry::likeUser` function can lead to loss of funds

Description

The LikeRegistry::likeUser function accepts ether via msg.value to facilitate the funds to the MultiSigWallet contract (corresponding to the match) when a match is being created. However, it lacks the check in case if the user sends more ether than required (i.e., 1 ether).

Impact

When a user sends a higher value of ether than what is necessary for the transaction (i.e., 1 ether) while executing the LikeRegistry::likeUser function, the contract processes the transaction without returning the excess ether to the user. This results in the user losing the excess ether, which is never refunded or handled by the contract.

Proof of Concept

  1. User executes the LikeRegistry::likeUser function passing the address of the liked user and attaching excessive ether amount.

  2. The excess ether remains stuck in the contract with no way to withdraw or refund it.

Proof of Code

Create a test file named as DatingDappTest.t.sol inside the test folder with the following content:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import {Test} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
contract DatingDappTest is Test {
SoulboundProfileNFT soulboundNFT;
LikeRegistry likeRegistry;
address bob = makeAddr("bob");
address alice = makeAddr("alice");
address owner = makeAddr("owner");
function setUp() public {
vm.startPrank(owner);
soulboundNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(soulboundNFT));
vm.stopPrank();
}
function testTransferExcessEtherWhileCallingLikeUser() public {
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 24, "ipfs://profileImage");
hoax(bob, 10 ether);
likeRegistry.likeUser{value: 10 ether}(alice); // Only 1 ether is required, but the remaining 9 will be stuck in the contract.
assertEq(address(likeRegistry).balance, 10 ether);
}
}

Recommended Mitigation

You could implement a check in the LikeRegistry::likeUser function to refund in case of overpayment:

+ if (msg.value > 1 ether) {
+ uint256 overpaidAmount = msg.value - 1 ether;
+ (bool success,) = payable(msg.sender).call{value: overpaidAmount}("");
+ require(success, "Transfer failed");
+ }
Updates

Appeal created

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

Users mistake, only impacting themselves.

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood 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.