diff --git a/api/src/lib.rs b/api/src/lib.rs index a2eba54..131d2d9 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -2391,6 +2391,25 @@ pub fn valid_email(addr: &str) -> bool { ) } +/// True if `nick` is a syntactically valid IRC nickname safe to introduce as a +/// pseudo-client: 1–30 chars, a letter or RFC "special" first (never a digit, +/// `-`, `#`, `:`, space or control), and only nick-legal chars after. Guards the +/// operator BOT ADD/CHANGE path so a bogus `:evil`/`#chan`/spaced nick can't be +/// minted into an IntroduceUser line. +pub fn valid_nick(nick: &str) -> bool { + let count = nick.chars().count(); + if count == 0 || count > 30 { + return false; + } + const SPECIAL: &str = "[]\\`_^{|}"; + let mut chars = nick.chars(); + let first = chars.next().unwrap(); + if !(first.is_ascii_alphabetic() || SPECIAL.contains(first)) { + return false; + } + nick.chars().all(|c| c.is_ascii_alphanumeric() || SPECIAL.contains(c) || c == '-') +} + /// 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 @@ -2973,4 +2992,21 @@ mod help_tests { assert!(!valid_email("@nolocal.com")); assert!(!valid_email("nodomain@")); } + + #[test] + fn valid_nick_guards_bot_names() { + assert!(valid_nick("Bendy")); + assert!(valid_nick("Guest12345")); + assert!(valid_nick("a-b_c`")); + assert!(valid_nick("[bot]")); + // Protocol-hostile or malformed nicks are rejected. + assert!(!valid_nick(":evil")); // colon would open a trailing param + assert!(!valid_nick("#chan")); + assert!(!valid_nick("has space")); + assert!(!valid_nick("bad\x02ctrl")); + assert!(!valid_nick("1leading")); + assert!(!valid_nick("-leading")); + assert!(!valid_nick("")); + assert!(!valid_nick(&"x".repeat(31))); + } } diff --git a/lang/de.json b/lang/de.json index 2761f88..188a18d 100644 --- a/lang/de.json +++ b/lang/de.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "Du bist jetzt der Gründer von \u0002{chan}\u0002 (übernommen von \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "Du hast dich nicht rechtzeitig identifiziert; du wurdest umbenannt.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Bitte beachte die Kanalregeln — {reason} Beim nächsten Mal wirst du gekickt.", - "Access denied — {what} is for services operators.": "Zugriff verweigert — {what} ist für Services-Operatoren." + "Access denied — {what} is for services operators.": "Zugriff verweigert — {what} ist für Services-Operatoren.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 ist kein gültiger Nickname." } \ No newline at end of file diff --git a/lang/es-ar.json b/lang/es-ar.json index cd81319..e623a36 100644 --- a/lang/es-ar.json +++ b/lang/es-ar.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "Ahora sos el fundador de \u0002{chan}\u0002 (heredado de \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "No te identificaste a tiempo; se te cambió el nombre.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Prestá atención a las reglas del canal — {reason} La próxima vez te van a expulsar.", - "Access denied — {what} is for services operators.": "Acceso denegado — {what} es solo para operadores de los servicios." + "Access denied — {what} is for services operators.": "Acceso denegado — {what} es solo para operadores de los servicios.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 no es un nick válido." } \ No newline at end of file diff --git a/lang/es.json b/lang/es.json index ee377df..4352991 100644 --- a/lang/es.json +++ b/lang/es.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "Ahora eres el fundador de \u0002{chan}\u0002 (heredado de \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "No te identificaste a tiempo; se te ha cambiado el nombre.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Ten en cuenta las reglas del canal — {reason} La próxima vez se te expulsará.", - "Access denied — {what} is for services operators.": "Acceso denegado — {what} es solo para operadores de servicios." + "Access denied — {what} is for services operators.": "Acceso denegado — {what} es solo para operadores de servicios.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 no es un nick válido." } \ No newline at end of file diff --git a/lang/fr.json b/lang/fr.json index 825e607..26eaec3 100644 --- a/lang/fr.json +++ b/lang/fr.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "Vous êtes maintenant le fondateur de \u0002{chan}\u0002 (hérité de \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "Vous ne vous êtes pas identifié à temps ; vous avez été renommé.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Veuillez respecter les règles du canal — {reason} La prochaine fois, vous serez expulsé.", - "Access denied — {what} is for services operators.": "Accès refusé — {what} est réservé aux opérateurs des services." + "Access denied — {what} is for services operators.": "Accès refusé — {what} est réservé aux opérateurs des services.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 n'est pas un pseudo valide." } \ No newline at end of file diff --git a/lang/pt-br.json b/lang/pt-br.json index a17532d..1a815da 100644 --- a/lang/pt-br.json +++ b/lang/pt-br.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "Você agora é o fundador de \u0002{chan}\u0002 (herdado de \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "Você não se identificou a tempo; seu apelido foi alterado.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Respeite as regras do canal — {reason} Da próxima vez você será expulso.", - "Access denied — {what} is for services operators.": "Acesso negado — {what} é para operadores de serviços." + "Access denied — {what} is for services operators.": "Acesso negado — {what} é para operadores de serviços.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 não é um apelido válido." } \ No newline at end of file diff --git a/lang/pt.json b/lang/pt.json index edd3a32..1082806 100644 --- a/lang/pt.json +++ b/lang/pt.json @@ -1333,5 +1333,6 @@ "You are now the founder of \u0002{chan}\u0002 (inherited from \u0002{account}\u0002).": "É agora o fundador de \u0002{chan}\u0002 (herdado de \u0002{account}\u0002).", "You didn't identify in time; you've been renamed.": "Não se identificou a tempo; o seu nome foi alterado.", "Please mind the channel rules — {reason} Next time you'll be kicked.": "Respeite as regras do canal — {reason} Da próxima vez será expulso.", - "Access denied — {what} is for services operators.": "Acesso negado — {what} é para operadores de serviços." + "Access denied — {what} is for services operators.": "Acesso negado — {what} é para operadores de serviços.", + "\u0002{nick}\u0002 isn't a valid nickname.": "\u0002{nick}\u0002 não é uma alcunha válida." } \ No newline at end of file diff --git a/modules/botserv/src/bot.rs b/modules/botserv/src/bot.rs index a118722..4f4e709 100644 --- a/modules/botserv/src/bot.rs +++ b/modules/botserv/src/bot.rs @@ -13,6 +13,10 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.notice(me, from.uid, "Syntax: BOT ADD [gecos]"); return; }; + if !echo_api::valid_nick(nick) { + ctx.notice(me, from.uid, t!(ctx, "\x02{nick}\x02 isn't a valid nickname.", nick = nick)); + return; + } let gecos = if args.len() > 5 { args[5..].join(" ") } else { "Service Bot".to_string() }; match db.bot_add(nick, user, host, &gecos) { Ok(()) => ctx.notice(me, from.uid, t!(ctx, "Bot \x02{nick}\x02 (\x02{user}@{host}\x02) added.", nick = nick, user = user, host = host)), @@ -24,6 +28,10 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.notice(me, from.uid, "Syntax: BOT CHANGE [user [host [gecos]]]"); return; }; + if !echo_api::valid_nick(newnick) { + ctx.notice(me, from.uid, t!(ctx, "\x02{nick}\x02 isn't a valid nickname.", nick = newnick)); + return; + } // Omitted fields keep the bot's current values. let Some(cur) = db.bots().into_iter().find(|b| b.nick.eq_ignore_ascii_case(old)) else { ctx.notice(me, from.uid, t!(ctx, "There's no bot named \x02{nick}\x02.", nick = old)); diff --git a/modules/gameserv/src/lib.rs b/modules/gameserv/src/lib.rs index c61130a..01c695d 100644 --- a/modules/gameserv/src/lib.rs +++ b/modules/gameserv/src/lib.rs @@ -390,7 +390,7 @@ impl GameServ { } let mut all: Vec<(String, Stats)> = map.into_iter().filter(|(_, s)| s.points() > 0).collect(); all.sort_by_key(|(_, s)| std::cmp::Reverse(s.points())); - push_tag(me, net, from.nick, &format!("GS$TOPSTART {}", gt.wire()), ctx); + push_tag(me, net, from.nick, from.account.unwrap_or(""), from.account.is_some(), &format!("GS$TOPSTART {}", gt.wire()), ctx); if all.is_empty() { ctx.notice(me, from.uid, t!(ctx, "Classement \x02{game}\x02 vide. Sois le premier à gagner !", game = gt.label())); return; @@ -401,7 +401,7 @@ impl GameServ { let pos = format!("{rank:2}"); let name = format!("{acc:<16}"); ctx.notice(me, from.uid, t!(ctx, "{pos}. {name} {tier} {pts} pts", pos = pos, name = name, tier = s.rank(), pts = s.points())); - push_tag(me, net, from.nick, &format!("GS$TOPROW {} {rank} {acc} {} {}", gt.wire(), s.rank(), s.points()), ctx); + push_tag(me, net, from.nick, from.account.unwrap_or(""), from.account.is_some(), &format!("GS$TOPROW {} {rank} {acc} {} {}", gt.wire(), s.rank(), s.points()), ctx); } } } @@ -409,18 +409,24 @@ impl GameServ { // ── web-sync pushes (free functions so they never borrow the GameServ state) ── // Client-only tag on a TAGMSG: invisible in chat, read only by the web client. -fn push_tag(me: &str, net: &dyn NetView, nick: &str, payload: &str, ctx: &mut ServiceCtx) { - if let Some(uid) = net.uid_by_nick(nick) { - ctx.actions.push(NetAction::Raw(format!("@+tchatou.fr/gs={} :{} TAGMSG {}", board::escape_tag(payload), me, uid))); +fn push_tag(me: &str, net: &dyn NetView, nick: &str, account: &str, registered: bool, payload: &str, ctx: &mut ServiceCtx) { + let Some(uid) = net.uid_by_nick(nick) else { return }; + // A registered player's board goes only to their own account: if they renamed + // mid-game and a stranger grabbed the old nick, deliver to nobody rather than + // leak the position to the nick-thief. Guests are keyed by a mutable nick + // already (casual games), so best-effort by nick is all we can do for them. + if registered && net.account_of(uid) != Some(account) { + return; } + ctx.actions.push(NetAction::Raw(format!("@+tchatou.fr/gs={} :{} TAGMSG {}", board::escape_tag(payload), me, uid))); } // The GS# state line for one side. `status_override` lets a challenge show as // "pending" to the challenger and "offer" to the target. fn push_one(g: &Game, viewer: Side, status_override: &str, me: &str, net: &dyn NetView, ctx: &mut ServiceCtx) { - let (nick, opp) = match viewer { - Side::A => (&g.nick_a, &g.nick_b), - Side::B => (&g.nick_b, &g.nick_a), + let (nick, opp, account, registered) = match viewer { + Side::A => (&g.nick_a, &g.nick_b, &g.acc_a, g.a_reg), + Side::B => (&g.nick_b, &g.nick_a, &g.acc_b, g.b_reg), }; let status = if status_override.is_empty() { g.status.wire() } else { status_override }; let res = match (g.status, g.result) { @@ -432,7 +438,7 @@ fn push_one(g: &Game, viewer: Side, status_override: &str, me: &str, net: &dyn N "GS# {} g={} s={} t={} me={} opp={} st={} r={}", g.id, g.gtype().wire(), g.encode(), g.glyph(g.turn), g.glyph(viewer), opp, status, res ); - push_tag(me, net, nick, &payload, ctx); + push_tag(me, net, nick, account, registered, &payload, ctx); } fn push_state(g: &Game, me: &str, net: &dyn NetView, ctx: &mut ServiceCtx) { @@ -450,7 +456,9 @@ fn push_stats(counters: &[(String, u64)], account: &str, to_nick: &str, me: &str _ => read_stats(counters, gt, account), }; let payload = format!("GS$ {} {} r={} p={} w={} l={} d={}", account, gt.wire(), s.rank(), s.points(), s.wins, s.losses, s.draws); - push_tag(me, net, to_nick, &payload, ctx); + // push_stats is only called for a registered side (a_reg/b_reg), so the + // account guard in push_tag always applies here. + push_tag(me, net, to_nick, account, true, &payload, ctx); } } diff --git a/modules/protocol/inspircd/src/lib.rs b/modules/protocol/inspircd/src/lib.rs index 192d457..3ffdcc8 100644 --- a/modules/protocol/inspircd/src/lib.rs +++ b/modules/protocol/inspircd/src/lib.rs @@ -195,7 +195,7 @@ impl Protocol for InspIrcd { // : PART [:reason] — a user leaving a channel. "PART" => match (source.as_deref(), tokens.next()) { (Some(uid), Some(chan)) if chan.starts_with('#') => { - vec![NetEvent::Part { uid: uid.to_string(), channel: chan.to_string(), reason: trailing(rest) }] + vec![NetEvent::Part { uid: uid.to_string(), channel: chan.to_string(), reason: reason(rest) }] } _ => vec![], }, @@ -204,7 +204,7 @@ impl Protocol for InspIrcd { let chan = tokens.next().unwrap_or(""); match tokens.next() { Some(uid) if chan.starts_with('#') => { - vec![NetEvent::Kicked { channel: chan.to_string(), uid: uid.to_string(), by: source.unwrap_or_default(), reason: trailing(rest) }] + vec![NetEvent::Kicked { channel: chan.to_string(), uid: uid.to_string(), by: source.unwrap_or_default(), reason: reason(rest) }] } _ => vec![], } @@ -291,7 +291,7 @@ impl Protocol for InspIrcd { _ => vec![], } } - "QUIT" => vec![NetEvent::Quit { uid: source.unwrap_or_default(), reason: trailing(rest) }], + "QUIT" => vec![NetEvent::Quit { uid: source.unwrap_or_default(), reason: reason(rest) }], // : KILL : — a user forcibly removed. We forget // them (or reintroduce, if it was one of our bots). "KILL" => match tokens.next() { @@ -683,6 +683,17 @@ fn trailing(rest: &str) -> String { } } +// A free-text reason param (PART/KICK/QUIT), which the ircd always sends as a +// `:`-prefixed trailing. Unlike `trailing()`, a MISSING reason yields "" rather +// than the last preceding word — so `KICK #c target` (no reason) doesn't record +// the target uid as the reason. +fn reason(rest: &str) -> String { + match rest.find(" :") { + Some(i) => rest[i + 2..].to_string(), + None => String::new(), + } +} + #[cfg(test)] mod tests { use super::*; @@ -691,6 +702,24 @@ mod tests { InspIrcd::new("services.test".into(), "Federated Services".into(), "42S".into(), "pw".into(), 1206, 1, "iHkBT".into()) } + // A KICK/PART/QUIT with no explicit `:reason` must record an EMPTY reason, + // not the last preceding word (a channel/target token). + #[test] + fn kick_without_reason_is_empty_not_the_target() { + let ev = proto().parse(":42SAAAAAB KICK #chan 0IRAAAAAB"); + assert!( + matches!(ev.as_slice(), [NetEvent::Kicked { reason, .. }] if reason.is_empty()), + "{ev:?}" + ); + let ev = proto().parse(":42SAAAAAB KICK #chan 0IRAAAAAB :flooding"); + assert!( + matches!(ev.as_slice(), [NetEvent::Kicked { reason, .. }] if reason == "flooding"), + "{ev:?}" + ); + let ev = proto().parse(":0IRAAAAAB QUIT"); + assert!(matches!(ev.as_slice(), [NetEvent::Quit { reason, .. }] if reason.is_empty()), "{ev:?}"); + } + // A remote nick change must surface as a NickChange for the source uid, or // nick-based commands act on a stale nick. #[test] diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 8fc484c..e8079c1 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1146,6 +1146,14 @@ impl Engine { self.bot_channels.clear(); self.network.clear_bots(); self.network.clear_servers(); // the burst re-introduces the server tree + // A re-link's burst re-introduces every user and channel, but the + // half-finished SASL exchanges and armed nick-enforcement timers from the + // dropped connection are orphaned — forget them so a re-link (were one ever + // added in-process) can't act on stale connection state. Today the process + // exits on uplink loss and systemd restarts it, so this is defensive. + self.sasl_sessions.clear(); + self.sasl_source.clear(); + self.pending_enforce.clear(); self.next_bot_index = 0; let mut out = vec![NetAction::Burst]; for svc in &self.services { diff --git a/src/engine/register.rs b/src/engine/register.rs index a1e68ba..9105ca1 100644 --- a/src/engine/register.rs +++ b/src/engine/register.rs @@ -60,6 +60,11 @@ impl Engine { return AuthorityStatus::Invalid; } } + // The verifiers are attacker-influenced if the authority is compromised: + // reject an absurd iteration count that would DoS later logins. + if !super::scram::verifier_ok(scram256) || (!scram512.is_empty() && !super::scram::verifier_ok(scram512)) { + return AuthorityStatus::Invalid; + } match self.db.provision_account(name, scram256, scram512, email) { Ok(()) => AuthorityStatus::Ok, Err(RegError::Exists) => AuthorityStatus::AlreadyExists, diff --git a/src/engine/scram.rs b/src/engine/scram.rs index 66353a9..46aaf8d 100644 --- a/src/engine/scram.rs +++ b/src/engine/scram.rs @@ -162,6 +162,16 @@ pub fn parse_verifier(encoded: &str) -> Option { }) } +// An externally supplied verifier (a gRPC Provision backfill) is untrusted for +// its cost parameter: a huge `i=` would make every later PBKDF2 verify burn a +// core — a compromised-authority DoS. Accept only a well-formed verifier whose +// iteration count is in a sane range (the floor matches the config minimum, the +// ceiling is a few multiples of the default so a real backfill still fits). +pub const MAX_VERIFIER_ITERATIONS: u32 = 5_000_000; +pub fn verifier_ok(encoded: &str) -> bool { + parse_verifier(encoded).is_some_and(|v| (1000..=MAX_VERIFIER_ITERATIONS).contains(&v.iterations)) +} + // --- exchange --- pub struct ClientFirst { @@ -351,4 +361,19 @@ mod tests { assert!(prove(PASSWORD).is_some(), "the real password authenticates against the authority's verifier"); assert!(prove("wrong").is_none(), "a wrong password is rejected"); } + + #[test] + fn verifier_ok_rejects_absurd_iteration_cost() { + // Derive one cheap verifier, then swap the `i=` to probe the range — never + // actually run PBKDF2 at an absurd count (that IS the DoS we're bounding). + let cheap = make_verifier(Hash::Sha256, "pw", 4096); + assert!(verifier_ok(&cheap)); + let with_iters = |i: &str| cheap.replacen("i=4096", &format!("i={i}"), 1); + // A huge iteration count (compromised-authority CPU DoS) is refused. + assert!(!verifier_ok(&with_iters("4294967295"))); + assert!(!verifier_ok(&with_iters(&(MAX_VERIFIER_ITERATIONS + 1).to_string()))); + // A too-cheap or malformed verifier is refused. + assert!(!verifier_ok(&with_iters("100"))); + assert!(!verifier_ok("not a verifier")); + } }