Skip to content

shun: defer add-time ident match when client's USER hasn't landed#93

Open
MrLenin wants to merge 1 commit into
evilnet:masterfrom
MrLenin:fix/shun-pre-user-ident-defer
Open

shun: defer add-time ident match when client's USER hasn't landed#93
MrLenin wants to merge 1 commit into
evilnet:masterfrom
MrLenin:fix/shun-pre-user-ident-defer

Conversation

@MrLenin
Copy link
Copy Markdown
Contributor

@MrLenin MrLenin commented Jun 2, 2026

Summary

Fixes a long-standing false-positive in do_shun (the retroactive
shun-apply loop run at SHUN add time) where the ident half of a
user@host mask was bypassed for any client mid-registration.

A guard at ircd/shun.c:221-223 (introduced in 2019 by f317973)
short-circuited the whole ident match on empty cli_user->username,
which is the case for every client between make_user() (allocates
cli_user on first input) and register_user() (populates
username from the USER command). Result: a shun like bob@* would
match every mid-registration client, and the server emitted a
spurious SNO_GLINE "Shun active for " oper notice
attributing the shun to a user whose ident doesn't (yet) match.

The fix inverts the guard: always defer pre-USER, except when
sh_user is the literal single bare * (the only shape we can prove
matches any ident). The explicit sh_user[1] == '\0' check is
needed so non-bare patterns that lead with * (e.g. *foo = "ends
with foo") don't sneak through the shortcut.

Behaviour preserved

  • *@host shuns still apply pre-USER (the original "no identd
    running" use case — Jobe's intent in the 2019 guard).

Behaviour fixed

  • bob@*, *foo@*, ?bob@*, ?*@*, and every other pattern with a
    specific ident half no longer false-positive against
    mid-registration clients.

No enforcement consequence

shun_lookup invoked from m_privmsg/etc. re-evaluates the shun
once the client's USER lands and its ident match runs unconditionally
there. This patch only fixes the add-time apply loop's mirror
behaviour — what it suppresses is the spurious oper-side notice for
the wrong user.

Test plan

Verified against an integration testnet (MrLenin/testnet) with
5 vitest cases covering:

  • Pre-USER client + bob@* mask → no false-positive (the bug)
  • Registered + non-matching ident → no false-positive (regression)
  • Registered + matching ident → matches (sanity)
  • Pre-USER + *foo@* mask → deferred (*foo is "ends with foo", not match-any)
  • Pre-USER + ?bob@* mask → deferred (?bob is "char-then-bob", not match-any)

All 5 pass against the patched build.

Add-time apply loop (do_shun) was bypassing the ident half of a
user@host shun for any client whose cli_user->username was still empty
— pre-USER clients in mid-registration where cli_user is allocated but
the USER command hasn't been parsed yet.  Guard at lines 221-223
(introduced 2019 in f317973) short-circuited the whole ident check on
empty username, so a shun like `bob@*` matched every mid-registration
client, and the server emitted a spurious `SNO_GLINE` "Shun active for
<victim>" oper notice attributing the shun to a user whose ident
doesn't (yet) match.

Fix: always defer pre-USER, except when sh_user is the literal single
bare `*` — the only shape we can prove matches any ident.  The
explicit `sh_user[1] == '\0'` check is needed so non-bare patterns
that lead with `*` (e.g. `*foo` = "ends with foo") don't sneak through
the shortcut.

This preserves the original "no identd running, mask must be `*@host`"
use case: a `*@1.2.3.4` shun still applies pre-USER.  All other shapes
(`bob@*`, `*foo@*`, `?bob@*`, `?*@*`, etc.) defer — and shun_lookup
invoked from m_privmsg/etc.  re-evaluates them once USER lands, so
deferral has no enforcement consequence.  The only artefact this
suppresses is the spurious oper notice for the wrong user.

The matching else-branch already handled the empty-username case
correctly (it only ran match() when username was non-empty), so the
fix is localised to the early-defer guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant