diff --git a/api/src/lib.rs b/api/src/lib.rs index 566abe8..8213295 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -2277,6 +2277,25 @@ pub fn valid_email(addr: &str) -> bool { ) } +/// The next free guest nick: `base` + an incrementing counter, skipping any nick +/// that is currently online or registered. An enforced/logged-out rename must not +/// land on a real user or a registered nick — that would either defeat the rename +/// (the target stays put) or collide with and kill an innocent occupant. Advances +/// `seq` past whatever it returns. +pub fn next_guest_nick(base: &str, seq: &mut u32, net: &dyn NetView, store: &dyn Store) -> String { + for _ in 0..100_000 { + let candidate = format!("{base}{seq}"); + *seq = seq.wrapping_add(1); + if net.uid_by_nick(&candidate).is_none() && !store.exists(&candidate) { + return candidate; + } + } + // Pathological: the whole range is taken. Fall back rather than loop forever. + let candidate = format!("{base}{seq}"); + *seq = seq.wrapping_add(1); + candidate +} + // Reject a look-alike / mixed-script registration name (nick or channel): the // impersonation trick that a message-content filter never sees at the registration // seam. Returns a rejection reason, or None if the name is fine. Genuine diff --git a/modules/nickserv/src/ghost.rs b/modules/nickserv/src/ghost.rs index 93bae09..06eda33 100644 --- a/modules/nickserv/src/ghost.rs +++ b/modules/nickserv/src/ghost.rs @@ -48,8 +48,7 @@ pub fn handle(me: &str, guest_nick: &str, guest_seq: &mut u32, from: &Sender, ar return; } Some(ghost) => { - let guest = format!("{guest_nick}{guest_seq}"); - *guest_seq = guest_seq.wrapping_add(1); + let guest = echo_api::next_guest_nick(guest_nick, guest_seq, net, &*db); ctx.force_nick(&ghost, &guest); ctx.notice(me, from.uid, format!("\x02{target}\x02 has been freed.")); } diff --git a/modules/nickserv/src/lib.rs b/modules/nickserv/src/lib.rs index a13811d..fe026d9 100644 --- a/modules/nickserv/src/lib.rs +++ b/modules/nickserv/src/lib.rs @@ -112,7 +112,7 @@ impl Service for NickServ { match cmd.as_deref() { Some("REGISTER") => register::handle(me, from, args, ctx, db), Some("IDENTIFY") | Some("ID") => identify::handle(me, from, args, ctx, db), - Some("LOGOUT") | Some("LOGOFF") => logout::handle(me, &self.guest_nick, &mut self.guest_seq, from, ctx), + Some("LOGOUT") | Some("LOGOFF") => logout::handle(me, &self.guest_nick, &mut self.guest_seq, from, ctx, net, db), Some("CERT") => cert::handle(me, from, args, ctx, db), Some("INFO") => info::handle(me, from, args, ctx, net, db), Some("ALIST") => alist::handle(me, from, ctx, db), diff --git a/modules/nickserv/src/logout.rs b/modules/nickserv/src/logout.rs index 3f4c53d..59aeb06 100644 --- a/modules/nickserv/src/logout.rs +++ b/modules/nickserv/src/logout.rs @@ -1,13 +1,12 @@ -use echo_api::{Sender, ServiceCtx}; +use echo_api::{NetView, Sender, ServiceCtx, Store}; // LOGOUT: log out and rename to a guest nick (prefix + a per-logout sequence). -pub fn handle(me: &str, guest_nick: &str, guest_seq: &mut u32, from: &Sender, ctx: &mut ServiceCtx) { +pub fn handle(me: &str, guest_nick: &str, guest_seq: &mut u32, from: &Sender, ctx: &mut ServiceCtx, net: &dyn NetView, db: &dyn Store) { if from.account.is_none() { ctx.notice(me, from.uid, "You're not logged in."); return; } - let guest = format!("{guest_nick}{guest_seq}"); - *guest_seq = guest_seq.wrapping_add(1); + let guest = echo_api::next_guest_nick(guest_nick, guest_seq, net, db); ctx.logout(from.uid); ctx.force_nick(from.uid, &guest); ctx.notice(me, from.uid, format!("You're now logged out. Your nick is now \x02{}\x02.", guest)); diff --git a/src/engine/tests.rs b/src/engine/tests.rs index a2b716c..7a53b81 100644 --- a/src/engine/tests.rs +++ b/src/engine/tests.rs @@ -418,6 +418,22 @@ e.handle(NetEvent::Privmsg { from: uid.into(), to: "42SAAAAAA".into(), text: "LOGOUT".into() }) } + // A guest rename must skip a nick that's already online, or it would collide + // with (and get) an innocent user instead of freeing the name. + #[test] + fn logout_skips_an_occupied_guest_nick() { + let mut e = engine_with("guestskip", "foo", "sesame"); + e.handle(NetEvent::UserConnect { uid: "000AAAAAB".into(), nick: "foo".into(), host: "h".into() , ip: "0.0.0.0".into() }); + e.handle(NetEvent::Privmsg { from: "000AAAAAB".into(), to: "42SAAAAAA".into(), text: "IDENTIFY sesame".into() }); + // Someone already holds Guest12345 (the first nick the counter would pick). + e.handle(NetEvent::UserConnect { uid: "000AAAAAC".into(), nick: "Guest12345".into(), host: "h".into() , ip: "0.0.0.0".into() }); + let out = logout(&mut e, "000AAAAAB"); + assert!( + out.iter().any(|a| matches!(a, NetAction::ForceNick { uid, nick } if uid == "000AAAAAB" && nick == "Guest12346")), + "logout skips the taken guest nick: {out:?}" + ); + } + // LOGOUT while not identified must not rename you or clear anything. #[test] fn logout_without_login_is_noop() {