DatingDapp

AI First Flight #6

Beginner FriendlyFoundrySolidityNFT

EXP

View results

Previous Next

Submission Details

Impact: low

Likelihood: low

Invalid

LikeRegistry::receive accepts plain ETH transfers that are credited to neither userBalances nor totalFees and that no function can withdraw, so ETH sent directly to the registry is locked forever, even after the userBalances fix

friedsocks233

Root + Impact

Description

Every wei the registry holds should be accounted for, either as a pending userBalances entry or as totalFees, so that it can leave through a match payout or through withdrawFees.
receive() (L83) accepts any ETH unconditionally and records nothing. The registry has only two outflows. The match payout (L65) is computed from userBalances. withdrawFees (L78) is capped at totalFees and blocked by L74 when it is 0. There is no sweep or rescue function, and OpenZeppelin's Ownable v5 adds only transferOwnership and renounceOwnership. ETH sent straight to the registry is therefore stranded permanently. Crediting msg.value in likeUser, the fix for the separately reported like-payment bug, does not touch this path.

// src/LikeRegistry.sol L82-L83: any ETH is accepted and nothing records it

/// @notice Allows the contract to receive ETH

@> receive() external payable {}

// src/LikeRegistry.sol L56-L58: match payouts are computed only from userBalances

uint256 totalRewards = matchUserOne + matchUserTwo;

uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;

uint256 rewards = totalRewards - matchingFees;

// src/LikeRegistry.sol L73-L78: the only other outflow pays exactly totalFees

function withdrawFees() external onlyOwner {

@> require(totalFees > 0, "No fees to withdraw");

uint256 totalFeesToWithdraw = totalFees;

totalFees = 0;

@> (bool success,) = payable(owner()).call{value: totalFeesToWithdraw}("");

Risk

Likelihood:

It happens when a user or integrator sends ETH to the registry with a plain transfer instead of calling likeUser. No protocol flow sends ETH to receive(), so this needs a sender mistake.
receive() succeeds silently, so the sender gets no revert to warn them.

Impact:

The sender loses the ETH. In the PoC, 1 ETH sent directly cannot be recovered: withdrawFees reverts No fees to withdraw for the owner, and again for a new owner after transferOwnership, and a later match does not sweep it.
The loss survives the userBalances fix. The PoC applies only that fix to a copy of LikeRegistry. On it, a match plus withdrawFees bring every tracked liability to 0, yet exactly the 1 ETH sent directly remains, and a second withdrawFees reverts. Other users' funds and the protocol's accounting are not affected.

Proof of Concept

The PoC deploys SoulboundProfileNFT and LikeRegistry. It also deploys FixedLikeRegistryDD11, a copy of LikeRegistry with only the like-crediting fix (userBalances[msg.sender] += msg.value;) applied and the same empty receive(). A storage-layout check confirms that the private totalFees is at slot 2.

On the real registry, a plain 1 ETH transfer succeeds and credits neither userBalances nor totalFees. withdrawFees then reverts for the owner, for a new owner after transferOwnership, and again after a full match.

The baseline shows that the fixed copy drains to zero after a match and withdrawFees. With one extra direct 1 ETH transfer, every tracked liability on the fixed copy reaches zero while exactly 1 ETH remains, and a second withdrawFees reverts.

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "forge-std/Test.sol";

import "../src/SoulboundProfileNFT.sol";

import "../src/LikeRegistry.sol";

import "../src/MultiSig.sol";

import "@openzeppelin/contracts/access/Ownable.sol";

/// @dev Copy of LikeRegistry with ONLY the main-bug fix applied

/// (`userBalances[msg.sender] += msg.value;` in likeUser). Storage layout and

/// every other line (including the empty `receive()`) are identical. Used to

/// show that the direct-transfer stranding survives the userBalances fix.

contract FixedLikeRegistryDD11 is Ownable {

struct Like {

address liker;

address liked;

uint256 timestamp;

}

SoulboundProfileNFT public profileNFT;

uint256 immutable FIXEDFEE = 10;

uint256 totalFees;

mapping(address => mapping(address => bool)) public likes;

mapping(address => address[]) public matches;

mapping(address => uint256) public userBalances;

constructor(address _profileNFT) Ownable(msg.sender) {

profileNFT = SoulboundProfileNFT(_profileNFT);

}

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;

userBalances[msg.sender] += msg.value; // <-- the main-bug fix, nothing else changed

if (likes[liked][msg.sender]) {

matches[msg.sender].push(liked);

matches[liked].push(msg.sender);

matchRewards(liked, msg.sender);

}

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;

MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);

(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");

require(success, "Transfer failed");

}

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

}

receive() external payable {}

}

contract PoC_DD11 is Test {

// Storage layout of LikeRegistry: slot0 Ownable._owner, slot1 profileNFT,

// FIXEDFEE is immutable (no slot), slot2 totalFees.

bytes32 constant TOTAL_FEES_SLOT = bytes32(uint256(2));

SoulboundProfileNFT nft;

LikeRegistry registry;

FixedLikeRegistryDD11 fixedRegistry;

address owner = makeAddr("owner");

address sender = makeAddr("mistakenSender");

address alice = makeAddr("alice");

address bob = makeAddr("bob");

function setUp() public {

nft = new SoulboundProfileNFT();

vm.startPrank(owner);

registry = new LikeRegistry(address(nft));

fixedRegistry = new FixedLikeRegistryDD11(address(nft));

vm.stopPrank();

vm.prank(alice);

nft.mintProfile("Alice", 25, "ipfs://alice");

vm.prank(bob);

nft.mintProfile("Bob", 27, "ipfs://bob");

vm.deal(sender, 10 ether);

vm.deal(alice, 10 ether);

vm.deal(bob, 10 ether);

}

function _totalFees(address reg) internal view returns (uint256) {

return uint256(vm.load(reg, TOTAL_FEES_SLOT));

}

/// Sanity: slot 1 is profileNFT and slot 0 is the owner, so slot 2 is totalFees.

function test_storageLayoutSanity() public view {

assertEq(address(uint160(uint256(vm.load(address(registry), bytes32(uint256(0)))))), owner);

assertEq(address(uint160(uint256(vm.load(address(registry), bytes32(uint256(1)))))), address(nft));

assertEq(registry.owner(), owner);

}

/// Main claim on the in-scope contract: a plain ETH transfer is accepted, credited

/// to nothing, and no outflow can return it.

function test_DirectEthToLikeRegistry_IsAcceptedButUnaccountedAndUnwithdrawable() public {

uint256 senderBefore = sender.balance;

// A user sends 1 ETH straight to the registry instead of calling likeUser.

vm.prank(sender);

(bool ok,) = address(registry).call{value: 1 ether}("");

assertTrue(ok, "receive() accepted the plain transfer");

assertEq(address(registry).balance, 1 ether, "registry now holds the 1 ETH");

assertEq(sender.balance, senderBefore - 1 ether, "sender lost 1 ETH");

assertEq(registry.userBalances(sender), 0, "not credited to userBalances");

assertEq(_totalFees(address(registry)), 0, "not credited to totalFees");

// The only owner outflow refuses to move it.

vm.prank(owner);

vm.expectRevert(bytes("No fees to withdraw"));

registry.withdrawFees();

// Changing owner (the only other admin surface from OZ Ownable) does not help.

address newOwner = makeAddr("newOwner");

vm.prank(owner);

registry.transferOwnership(newOwner);

vm.prank(newOwner);

vm.expectRevert(bytes("No fees to withdraw"));

registry.withdrawFees();

// A match on the in-scope contract does not sweep it either.

vm.prank(alice);

registry.likeUser{value: 1 ether}(bob);

vm.prank(bob);

registry.likeUser{value: 1 ether}(alice);

assertEq(_totalFees(address(registry)), 0, "match added no fees");

vm.prank(newOwner);

vm.expectRevert(bytes("No fees to withdraw"));

registry.withdrawFees();

// The directly-sent 1 ETH is still there (plus the like payments of the main bug).

assertGe(address(registry).balance, 1 ether, "direct ETH still stranded");

emit log_named_decimal_uint("registry balance after all outflows tried", address(registry).balance, 18);

}

/// Baseline: with ONLY the userBalances fix, like payments are fully drained by

/// match + withdrawFees, and the registry ends at zero.

function test_Baseline_FixedRegistry_DrainsToZeroWithoutDirectTransfer() public {

vm.prank(alice);

fixedRegistry.likeUser{value: 1 ether}(bob);

vm.prank(bob);

fixedRegistry.likeUser{value: 1 ether}(alice);

// 1.8 ETH went to the multisig, 0.2 ETH sits as fees.

assertEq(_totalFees(address(fixedRegistry)), 0.2 ether, "slot 2 holds totalFees");

assertEq(address(fixedRegistry).balance, 0.2 ether);

uint256 ownerBefore = owner.balance;

vm.prank(owner);

fixedRegistry.withdrawFees();

assertEq(owner.balance - ownerBefore, 0.2 ether);

assertEq(address(fixedRegistry).balance, 0, "baseline: nothing left behind");

}

/// Limb 2: the stranding survives the userBalances fix. Same flow as the baseline

/// plus one direct 1 ETH transfer: every outflow is exhausted and exactly 1 ETH remains.

function test_DirectEthStillStranded_AfterUserBalancesFix() public {

vm.prank(sender);

(bool ok,) = address(fixedRegistry).call{value: 1 ether}("");

assertTrue(ok);

assertEq(fixedRegistry.userBalances(sender), 0, "fixed likeUser path does not touch receive()");

vm.prank(alice);

fixedRegistry.likeUser{value: 1 ether}(bob);

vm.prank(bob);

fixedRegistry.likeUser{value: 1 ether}(alice);

// Match paid only tracked balances: 1.8 ETH out, 0.2 ETH fees; the direct 1 ETH untouched.

assertEq(address(fixedRegistry).balance, 1.2 ether);

assertEq(_totalFees(address(fixedRegistry)), 0.2 ether, "direct ETH not counted as fees");

uint256 ownerBefore = owner.balance;

vm.prank(owner);

fixedRegistry.withdrawFees();

assertEq(owner.balance - ownerBefore, 0.2 ether, "owner can only take tracked fees");

// All liabilities are zero, yet the registry still holds the 1 ETH.

assertEq(_totalFees(address(fixedRegistry)), 0);

assertEq(fixedRegistry.userBalances(alice), 0);

assertEq(fixedRegistry.userBalances(bob), 0);

assertEq(fixedRegistry.userBalances(sender), 0);

assertEq(address(fixedRegistry).balance, 1 ether, "exactly the direct transfer is stranded");

vm.prank(owner);

vm.expectRevert(bytes("No fees to withdraw"));

fixedRegistry.withdrawFees();

emit log_named_decimal_uint("untracked ETH stranded in fixed registry", address(fixedRegistry).balance, 18);

}

Run from the repository root, with the file saved as test/PoC_DD-11.t.sol:

FOUNDRY_FFI=false FOUNDRY_DISABLE_NIGHTLY_WARNING=1 forge test --match-path test/PoC_DD-11.t.sol -vv

Result: Suite result: ok. 4 passed; 0 failed; 0 skipped. The logs show untracked ETH stranded in fixed registry: 1.000000000000000000.

Recommended Mitigation

Remove receive(). No protocol flow needs it, and without it (and with no fallback) plain transfers revert instead of being stranded. If the contract must accept ETH this way, add an onlyOwner sweep limited to address(this).balance minus totalFees and the outstanding userBalances.

--- a/src/LikeRegistry.sol

+++ b/src/LikeRegistry.sol

@@ -78,7 +78,4 @@

(bool success,) = payable(owner()).call{value: totalFeesToWithdraw}("");

require(success, "Transfer failed");

}

- /// @notice Allows the contract to receive ETH

- receive() external payable {}

}

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago

Submission Judgement Published

Invalidated

Reason: Incorrect statement

Rewards breakdown

This contest has a flat reward structure with the following payouts:

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being reviewed by our AI judge. Results will be available in a few minutes.

View all submissions

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!