Reject uid-form values as a topic setter or cooldown key (require a nick/mask)

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) {

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."),

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 <chan> <chanTS> <topicTS> [setby] :<topic>. The optional
-                        // setby (index 3) is the author's nick!ident@host; a ':'-prefixed
+                        // setby (index 3) is the author's nick!ident@host. Reject a
-                        // token there is the trailing topic, meaning no setby was sent.
+                        // ':'-prefixed token (that's the trailing topic — no setby sent)
-                        let setby = a.get(3).filter(|s| !s.starts_with(':')).map(|s| s.to_string()).unwrap_or_default();
+                        // 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
+                // The setby field must be a nick / nick!ident@host — never a uid. Omit it
-                // we don't know the author, omit it so the ircd credits the source's
+                // (the ircd then credits the source's nick) when we have no author, or a
-                // nick instead of printing our raw source uid.
+                // stale uid-form value (a nick never starts with a digit).
-                if setter.is_empty() {
+                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]

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."),