From 61d633ebf01309c0d656c77a663f6088ca8e409a Mon Sep 17 00:00:00 2001 From: MrLenin <909621+MrLenin@users.noreply.github.com> Date: Sat, 6 Jun 2026 03:08:31 -0400 Subject: [PATCH 1/3] proto-p10: GetUserN_silent variant for callers that legitimately probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factor the GetUserN numeric-to-userNode lookup into a static GetUserN_impl(numeric, quiet) helper, expose both the original GetUserN (noisy — keeps WARNING for every failure mode) and a new GetUserN_silent (suppresses all warnings on lookup miss). Flip the BX P handler's two probe lookups (cmd_bouncer_transfer: both old_primary and new_node) to the silent variant. Probing is the whole point — the old client may already be gone, and new_node is normally absent in the swap-path case. Command handlers and protocol parsers (opserv command targets, FAKEHOST/MARK/etc. dispatchers, channel-mode victims) keep using the noisy GetUserN — a miss there is a real protocol bug or config issue worth surfacing. mod-snoop / mod-track stay on the noisy variant too (those modules are off by default and operators who enable them want the misses). Symptom this addresses: noisy networks running the fork's bouncer subsystem were seeing repeated x3 warning: GetUserN(GkAAp) couldn't find user! snotices to O3 channels from the BX P probe path. Co-Authored-By: Claude Opus 4.7 --- src/proto-p10.c | 47 +++++++++++++++++++++++++++++++++++++++-------- src/proto.h | 1 + 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/proto-p10.c b/src/proto-p10.c index 44a7cb90..214669ba 100644 --- a/src/proto-p10.c +++ b/src/proto-p10.c @@ -388,8 +388,13 @@ GetServerN(const char *numeric) } } -struct userNode* -GetUserN(const char *numeric) /* using numeric */ +/* Shared internal numeric-to-userNode lookup. Wraps both the noisy + * GetUserN (callers expect success — a miss is bug-worthy) and the + * quiet GetUserN_silent (callers know the target can legitimately be + * absent — e.g., snoop/track on transient targets, BX P probing for + * the alias/primary pair). */ +static struct userNode * +GetUserN_impl(const char *numeric, int quiet) { struct userNode *un; struct server *s; @@ -397,26 +402,46 @@ GetUserN(const char *numeric) /* using numeric */ switch (strlen(numeric)) { default: - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric); return NULL; case 5: slen = 2; ulen = 3; break; case 4: slen = 1; ulen = 3; break; case 3: slen = 1; ulen = 2; break; case 2: case 1: case 0: - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric); return NULL; } if (!(s = servers_num[base64toint(numeric, slen)])) { - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen); return NULL; } n = base64toint(numeric+slen, ulen) & s->num_mask; if (!(un = s->users[n])) { - log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric); + if (!quiet) + log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric); } return un; } +struct userNode* +GetUserN(const char *numeric) /* using numeric */ +{ + return GetUserN_impl(numeric, 0); +} + +/* Same as GetUserN but suppresses all warnings on lookup miss. Use + * for callers that legitimately probe numerics which may not exist + * (snoop / track tracking transient PRIVMSG/NOTICE targets, BX P + * lookup probes during alias reconciliation, etc.). */ +struct userNode* +GetUserN_silent(const char *numeric) +{ + return GetUserN_impl(numeric, 1); +} + extern struct userNode *opserv; static void check_ctcp(struct userNode *user, struct userNode *bot, char *text, UNUSED_ARG(int server_qualified)) @@ -1735,8 +1760,14 @@ static CMD_FUNC(cmd_bouncer_transfer) if (argc < 6) return 0; - old_primary = GetUserN(argv[2]); - new_node = GetUserN(argv[3]); + /* Both lookups can legitimately miss — old_primary may have + * already been cleaned up by a prior event, new_node is + * normally absent (the swap path is the common case) but + * may exist for the in-place-conversion / merge case below. + * Use the silent variant so neither probe spams the snoop + * channel. */ + old_primary = GetUserN_silent(argv[2]); + new_node = GetUserN_silent(argv[3]); if (!old_primary) return 1; /* Already gone, nothing to do */ diff --git a/src/proto.h b/src/proto.h index 75a5e249..cf272044 100644 --- a/src/proto.h +++ b/src/proto.h @@ -84,6 +84,7 @@ struct cManagerNode #ifdef WITH_PROTOCOL_P10 struct server* GetServerN(const char *numeric); struct userNode* GetUserN(const char *numeric); +struct userNode* GetUserN_silent(const char *numeric); #endif /* Basic protocol parsing support. */ From df47a295f710e8ac92d98294d544258f230279fc Mon Sep 17 00:00:00 2001 From: MrLenin <909621+MrLenin@users.noreply.github.com> Date: Sat, 6 Jun 2026 03:09:01 -0400 Subject: [PATCH 2/3] proto-p10: BX P merge case for in-place conversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original cmd_bouncer_transfer BX P handler logged a warning and bailed when both old_primary and new_node existed locally. Modern fork peers can emit BX P in that exact shape when an N-introduced client is being absorbed into an existing primary on the same account — typically because we received the would-be-alias's N before its BX C in a burst, so both userNodes landed in our tables. Add a merge branch gated on same-handle: if both nodes exist AND share handle_info, delete old_primary's userNode via DelUser(announce=0) — channels, dict entry, oper list, etc. all get cleaned up by the standard DelUser path without a network QUIT/KILL. The surviving identity is new_node. The mismatched-handle case keeps the original "log and ignore" behaviour — we don't silently merge across unrelated accounts. Co-Authored-By: Claude Opus 4.7 --- src/proto-p10.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/proto-p10.c b/src/proto-p10.c index 214669ba..c887c12c 100644 --- a/src/proto-p10.c +++ b/src/proto-p10.c @@ -1772,16 +1772,43 @@ static CMD_FUNC(cmd_bouncer_transfer) if (!old_primary) return 1; /* Already gone, nothing to do */ - /* Numeric swap: move old_primary's numeric routing to the new - * server/slot. The userNode keeps its nick, clients dict entry, - * handle_info, channels — only the P10 numeric changes. + /* Two BX P shapes need handling: * - * Nefarious never bursts aliases as N tokens (they're introduced - * via BX C only), so X3 won't have a node for the alias numeric. - * If new_node somehow exists (shouldn't happen), log and ignore. */ + * 1. **Numeric swap (new_node absent)** — classic promote-or- + * transfer. Move old_primary's numeric routing to the new + * server/slot. The userNode keeps its nick, clients dict + * entry, handle_info, channels — only the P10 numeric changes. + * This is the case the original handler was written for, and + * is what fires when nefarious never bursts an alias to us. + * + * 2. **In-place conversion / merge (new_node exists with same + * account)** — modern fork peers emit BX P for the case where + * an N-introduced client (old) is being absorbed into an + * existing primary (new), e.g. burst-ordering caused us to + * receive N for the would-be-alias before its BX C. Both + * nodes exist on X3. We merge: delete old_primary's + * userNode (channels and dict entry cleaned up via DelUser, + * no QUIT broadcast), keep new_node intact as the surviving + * identity. Gate strictly on same-handle to avoid + * accidentally merging unrelated collisions. + * + * 3. **new_node exists but different/no handle** — genuinely + * unexpected. Keep the original "log and ignore" behaviour + * so we don't silently corrupt state across an account + * mismatch. */ if (new_node) { + if (old_primary->handle_info + && old_primary->handle_info == new_node->handle_info) { + /* Merge: old absorbed into new. DelUser with announce=0 + * suppresses both QUIT and KILL emission — this is + * internal cleanup, the network isn't supposed to see + * the alias's identity leave. */ + DelUser(old_primary, NULL, 0, "Bouncer transfer"); + return 1; + } log_module(MAIN_LOG, LOG_WARNING, - "BX P: new_node %s already exists as %s — ignoring promote", + "BX P: new_node %s already exists as %s with mismatched " + "handle — ignoring promote", argv[3], new_node->nick); return 1; } From b4d44c591b3a773eab4479c22936eaddf3d53eda Mon Sep 17 00:00:00 2001 From: MrLenin <909621+MrLenin@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:16:13 -0400 Subject: [PATCH 3/3] proto-p10: transfer memberships to survivor in BX P merge cmd_bouncer_transfer's same-handle merge branch (BX P where new_node already exists with old_primary's handle_info) called DelUser on old_primary without first moving its channel memberships to new_node. Both nefarious implementations of the both-exist case transfer memberships before killing the old node (upstream m_bouncer_transfer.c:88-113, fork bouncer_session.c:7070-7078); X3's merge branch skipped that step. Reachable in steady state via cross-server BOUNCER RESUME: old_primary is a ghost holding all of the session's channels, new_node is a fresh client with none. DelUser's own channel-removal loop calls DelChannelUser(..., NULL, 0), and DelChannelUser's tail destroys an unregistered channel when the removal empties it. With no prior transfer, any unregistered channel where old_primary was the only member from X3's point of view got destroyed mid-merge, even though the surviving identity (and the rest of the network) was still in it. Fix: before DelUser, walk old_primary's channel list the same way DelUser's own loop does (always pop the last element, since Add/DelChannelUser mutate both the channel's member list and the user's channel list). For each channel, add new_node with the same modes/oplevel if it isn't already a member, then remove old_primary with no announce. Doing this before DelUser means old_primary's channel list is already empty when DelUser runs, so its loop body never executes and never risks the auto-destroy path. AddChannelUser unconditionally fires call_join_funcs (chanserv join-time hooks) -- there's no hook-free "just add the membership" primitive, and the numeric-swap path above never touches channel membership at all so there's no hook-free both-exist precedent to follow instead. Accepted as a known side effect since membership state correctness is the priority; it does not emit anything on the wire (irc_join only fires for IsLocal users, i.e. X3's own service bots, never for a bounced network client). Co-Authored-By: Claude Opus 5 (1M context) --- src/proto-p10.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/src/proto-p10.c b/src/proto-p10.c index c887c12c..28b546fc 100644 --- a/src/proto-p10.c +++ b/src/proto-p10.c @@ -1799,10 +1799,68 @@ static CMD_FUNC(cmd_bouncer_transfer) if (new_node) { if (old_primary->handle_info && old_primary->handle_info == new_node->handle_info) { - /* Merge: old absorbed into new. DelUser with announce=0 - * suppresses both QUIT and KILL emission — this is - * internal cleanup, the network isn't supposed to see - * the alias's identity leave. */ + /* Merge: old absorbed into new. Before deleting + * old_primary, transfer its channel memberships onto + * new_node. This mirrors nefarious's own both-exist + * swap semantics (upstream m_bouncer_transfer.c:88-113, + * fork bouncer_session.c:7070-7078): the ghost's + * channels must survive on the merged identity. + * + * Ordering is load-bearing: this MUST run before + * DelUser(). DelUser's own cleanup loop + * (DelChannelUser(..., NULL, 0)) lets an unregistered + * channel self-destruct when its last member leaves + * (see DelChannelUser's tail, hash.c). If old_primary + * were deleted first while still holding memberships + * new_node lacks, any unregistered channel where + * old_primary was the only member in X3's view would + * be destroyed here — even though the surviving + * identity (and the rest of the network) is still in + * it. Transferring first means old_primary's channel + * list is already empty by the time DelUser runs, so + * that loop body never executes. + * + * Walk old_primary->channels the same way DelUser's + * loop does: always pop the last element, since + * AddChannelUser/DelChannelUser mutate both the + * channel's member list and the user's channel list + * out from under us. */ + while (old_primary->channels.used > 0) { + struct modeNode *mn = old_primary->channels.list[old_primary->channels.used - 1]; + struct chanNode *chan = mn->channel; + + if (!GetUserMode(chan, new_node)) { + /* AddChannelUser() only fires irc_join() when + * the joining user IsLocal() (i.e. an X3 + * service bot) — new_node here is always a + * network user, so no JOIN hits the wire. It + * unconditionally runs call_join_funcs() + * though, which fires the same on-join hooks + * (chanserv presence/ban/oplevel bookkeeping) + * a real join would. The numeric-swap path + * above never touches channel membership at + * all, so there's no existing both-exist + * precedent that's hook-free; there's no + * lower-level "add to channel, no hooks" + * primitive to reach for instead. Correctness + * of membership state takes priority, so we + * accept the hook firing here as a known + * side effect rather than leaving the ghost's + * channels to be silently dropped. */ + struct modeNode *new_mn = AddChannelUser(new_node, chan); + new_mn->modes = mn->modes; + new_mn->oplevel = mn->oplevel; + } + + /* No announce (reason NULL), same call shape + * DelUser's own loop uses: this is internal + * bookkeeping, not a real part. */ + DelChannelUser(old_primary, chan, NULL, 0); + } + + /* DelUser with announce=0 suppresses both QUIT and KILL + * emission — this is internal cleanup, the network + * isn't supposed to see the alias's identity leave. */ DelUser(old_primary, NULL, 0, "Bouncer transfer"); return 1; }