diff --git a/modules/chanserv/src/lib.rs b/modules/chanserv/src/lib.rs index 7ba5177..b23b3ff 100644 --- a/modules/chanserv/src/lib.rs +++ b/modules/chanserv/src/lib.rs @@ -193,7 +193,8 @@ impl Service for ChanServ { ctx.notice(me, from.uid, t!(ctx, " Email : {email}", email = info.email)); } ctx.notice(me, from.uid, t!(ctx, " Registered : {when}", when = echo_api::human_time(info.ts))); - if !info.topic_setter.is_empty() { + // Only a real nick/mask, never a bare uid (nicks never start with a digit). + if !info.topic_setter.is_empty() && !info.topic_setter.starts_with(|c: char| c.is_ascii_digit()) { ctx.notice(me, from.uid, t!(ctx, " Topic by : \x02{who}\x02", who = info.topic_setter)); } if let Some(s) = db.channel_suspension(chan) { diff --git a/modules/helpserv/src/request.rs b/modules/helpserv/src/request.rs index de0a18f..f000616 100644 --- a/modules/helpserv/src/request.rs +++ b/modules/helpserv/src/request.rs @@ -9,7 +9,9 @@ pub fn handle(me: &str, from: &Sender, rest: &[&str], ctx: &mut ServiceCtx, net: } let requester = from.account.unwrap_or(from.nick); // Rate-limit on the real host, not the spoofable nick (see REPORT). - let cooldown_key = net.host_of(from.uid).unwrap_or(from.uid); + // Rate-limit by host; fall back to the nick, never the uid (a uid is per-connection, + // so keying on it would reset the cooldown on every reconnect). + let cooldown_key = net.host_of(from.uid).unwrap_or(from.nick); match db.help_request(requester, cooldown_key, &message) { Some(id) => ctx.notice(me, from.uid, t!(ctx, "Thanks — your request (\x02#{id}\x02) is in the queue. A staff member will be with you.", id = id)), None => ctx.notice(me, from.uid, "You just opened a ticket — please wait a moment before opening another."), diff --git a/modules/protocol/inspircd/src/lib.rs b/modules/protocol/inspircd/src/lib.rs index 184334c..1d57736 100644 --- a/modules/protocol/inspircd/src/lib.rs +++ b/modules/protocol/inspircd/src/lib.rs @@ -269,9 +269,15 @@ impl Protocol for InspIrcd { match (source.as_deref(), a.first()) { (Some(src), Some(chan)) if !src.starts_with(self.sid.as_str()) && chan.starts_with('#') => { // FTOPIC [setby] :. The optional - // setby (index 3) is the author's nick!ident@host; a ':'-prefixed - // token there is the trailing topic, meaning no setby was sent. - let setby = a.get(3).filter(|s| !s.starts_with(':')).map(|s| s.to_string()).unwrap_or_default(); + // setby (index 3) is the author's nick!ident@host. Reject a + // ':'-prefixed token (that's the trailing topic — no setby sent) + // AND a digit-leading one: a nick never starts with a digit, so + // that is a bare uid the ircd echoed back from a prior push, not a + // real author, and a uid is meaningless once its owner reconnects. + let setby = a.get(3) + .filter(|s| !s.starts_with(':') && !s.starts_with(|c: char| c.is_ascii_digit())) + .map(|s| s.to_string()) + .unwrap_or_default(); vec![NetEvent::TopicChange { channel: chan.to_string(), setter: src.to_string(), topic: trailing(rest), setby }] } _ => vec![], @@ -547,10 +553,10 @@ impl Protocol for InspIrcd { // always accepts it; sourced from the given pseudoclient. NetAction::Topic { from, channel, topic, setter } => { let now = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(self.ts); - // The setby field must be a nick / nick!ident@host — never a uid. When - // we don't know the author, omit it so the ircd credits the source's - // nick instead of printing our raw source uid. - if setter.is_empty() { + // The setby field must be a nick / nick!ident@host — never a uid. Omit it + // (the ircd then credits the source's nick) when we have no author, or a + // stale uid-form value (a nick never starts with a digit). + if setter.is_empty() || setter.starts_with(|c: char| c.is_ascii_digit()) { vec![format!(":{} FTOPIC {} 1 {} :{}", from, channel, now, topic)] } else { vec![format!(":{} FTOPIC {} 1 {} {} :{}", from, channel, now, setter, topic)] @@ -1099,6 +1105,20 @@ mod tests { let head = without[0].split(" :").next().unwrap(); assert!(head.split(' ').next_back().unwrap().chars().all(|c| c.is_ascii_digit()), "unknown author -> no setby token (ends with topicTS), no uid: {without:?}"); + + // A stale uid-form setter (the ircd echoing back a prior bad push) is omitted too. + let uidform = proto().serialize(&NetAction::Topic { from: "42SAAAAAA".into(), channel: "#c".into(), topic: "hi".into(), setter: "00DB00000".into() }); + let head = uidform[0].split(" :").next().unwrap(); + assert!(head.split(' ').next_back().unwrap().chars().all(|c| c.is_ascii_digit()), + "uid setter omitted, never emitted: {uidform:?}"); + } + + // The ircd can echo back a bare uid in the setby slot (a stale value from an + // earlier push); we must not trust it as an author. + #[test] + fn ftopic_rejects_a_uid_in_the_setby_slot() { + let ev = proto().parse(":001 FTOPIC #chan 1 2 00DB00000 :hello"); + assert!(matches!(ev.as_slice(), [NetEvent::TopicChange { setby, .. }] if setby.is_empty()), "uid setby rejected: {ev:?}"); } #[test] diff --git a/modules/reportserv/src/report.rs b/modules/reportserv/src/report.rs index d6ab407..f81cf1a 100644 --- a/modules/reportserv/src/report.rs +++ b/modules/reportserv/src/report.rs @@ -14,7 +14,9 @@ pub fn handle(me: &str, from: &Sender, rest: &[&str], ctx: &mut ServiceCtx, net: let reporter = from.account.unwrap_or(from.nick); // Rate-limit on the real host, not the (spoofable) nick a flooder could cycle to // reset the cooldown. Anonymous reports still work; the host anchors the limit. - let cooldown_key = net.host_of(from.uid).unwrap_or(from.uid); + // Rate-limit by host; fall back to the nick, never the uid (a uid is per-connection, + // so keying on it would reset the cooldown on every reconnect). + let cooldown_key = net.host_of(from.uid).unwrap_or(from.nick); match db.report_file(reporter, cooldown_key, target, &reason) { Some(id) => ctx.notice(me, from.uid, t!(ctx, "Thanks — your report (\x02#{id}\x02) about \x02{target}\x02 has been sent to the staff.", id = id, target = target)), None => ctx.notice(me, from.uid, "You just filed a report — please wait a moment before filing another."),