Harden deferred audit LOWs: provisioned-verifier cost cap, BOT-nick validation, gameserv board-delivery account guard, empty KICK/PART/QUIT reason parse, and relink SASL/enforce cleanup
All checks were successful
CI / check (push) Successful in 5m4s
All checks were successful
CI / check (push) Successful in 5m4s
This commit is contained in:
parent
1b569fdfa3
commit
4027242c1e
13 changed files with 144 additions and 19 deletions
|
|
@ -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)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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."
|
||||
}
|
||||
|
|
@ -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 <nick> <user> <host> [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 <oldnick> <newnick> [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));
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -195,7 +195,7 @@ impl Protocol for InspIrcd {
|
|||
// :<uid> PART <chan> [: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) }],
|
||||
// :<src> KILL <uid> :<reason> — 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]
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -162,6 +162,16 @@ pub fn parse_verifier(encoded: &str) -> Option<Verifier> {
|
|||
})
|
||||
}
|
||||
|
||||
// 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"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue