#3876: review — Zotero owner check streams the socket table, and an unreadable table refuses

The Linux same-user check read /proc/net/tcp{,6} through a 64 MiB take(). Hitting the cap
was silent, so a local user binding ~450k loopback listeners pushed the connection rows
past it; both uids came back unknown and the "trust loopback" arm let their fake Zotero
listener on 23116 through. A truncated last line could also glue to the next header.

Now each table is streamed line by line (BufRead, nothing kept, no total cap, lines capped
at 4 KiB) and stops once both rows are found. Any read error refuses. "Unknown, trust
loopback" remains only for a listener absent from a table read in full (WSL2, proxies).
Tests: rows after 70 MiB of filler, and a read error mid-table.

rich.rs: document that the MAX_RUNS cap still allows one first run per paragraph (bounded
by MAX_PARAS).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

crates/zotero/src/peer.rs

@@ -35,23 +35,25 @@ pub(crate) fn check(stream: &TcpStream) -> Result<(), ZoteroError> {
 }
 
 /// Refuse a listener at `peer` that the kernel's socket tables say belongs to another user.
+/// A table that can't be read in full (I/O error) is a refusal, never "unknown".
 #[cfg(any(target_os = "linux", target_os = "android"))]
 fn owner_check(local: SocketAddr, peer: SocketAddr) -> Result<(), ZoteroError> {
-    use std::io::Read;
-    /// Longest socket table read (a busy server's table is a few MB).
-    const MAX_TABLE: u64 = 64 * 1024 * 1024;
-    let mut table = String::new();
+    use std::io::BufReader;
+    let mut rows = Rows::default();
     for path in ["/proc/net/tcp", "/proc/net/tcp6"] {
-        if let Ok(f) = std::fs::File::open(path) {
-            let mut part = String::new();
-            if f.take(MAX_TABLE).read_to_string(&mut part).is_ok() {
-                table.push_str(&part);
-            }
+        // A missing file (no IPv6) is an absent table; any other failure is a refusal.
+        match std::fs::File::open(path) {
+            Ok(f) => scan_table(BufReader::new(f), local, peer, &mut rows)?,
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+            Err(e) => return Err(unreadable(&e)),
+        }
+        if rows.done() {
+            break;
         }
     }
     // Our uid when our own socket's row is missing: `/proc/self` belongs to this process's user.
     let me = std::fs::metadata("/proc/self").ok().map(|m| std::os::unix::fs::MetadataExt::uid(&m));
-    same_owner(&table, local, peer, me)
+    decide(&rows, peer, me)
 }
 
 /// Windows, macOS, the BSDs: no same-user check (no socket table to read without OS calls this
@@ -61,16 +63,39 @@ fn owner_check(_local: SocketAddr, _peer: SocketAddr) -> Result<(), ZoteroError>
     Ok(())
 }
 
-/// From a `/proc/net/tcp`-format socket table: refuse when the socket at `peer` connected to
-/// `local` (the listener's side) belongs to another user than ours. Our uid is our own socket's
-/// row, else `me` (this process's). When the listener isn't listed (or the table couldn't be
-/// read, or our uid is unknown) its owner is unknown and the loopback rule decides.
-#[cfg_attr(not(any(target_os = "linux", target_os = "android")), allow(dead_code))]
-pub(crate) fn same_owner(table: &str, local: SocketAddr, peer: SocketAddr, me: Option<u32>) -> Result<(), ZoteroError> {
+/// The uids found for the two rows of one connection.
+#[derive(Default)]
+struct Rows {
+    ours: Option<u32>,
+    theirs: Option<u32>,
+}
+
+impl Rows {
+    fn done(&self) -> bool {
+        self.ours.is_some() && self.theirs.is_some()
+    }
+}
+
+fn unreadable(e: &std::io::Error) -> ZoteroError {
+    ZoteroError::Refused(format!("couldn't read the system's socket table ({e}), so the program on the Zotero port can't be checked"))
+}
+
+/// Stream a `/proc/net/tcp`-format table line by line (nothing is kept, so the table's size
+/// doesn't matter and nothing is truncated), recording the uids of the connection's two rows.
+/// Stops once both are found. Any read error is a refusal: a table we couldn't finish reading
+/// must never count as "the listener isn't listed".
+fn scan_table(mut table: impl std::io::BufRead, local: SocketAddr, peer: SocketAddr, rows: &mut Rows) -> Result<(), ZoteroError> {
     let key = |a: SocketAddr| (a.ip().to_canonical(), a.port());
     let (local, peer) = (key(local), key(peer));
-    let (mut ours, mut theirs) = (None, None);
-    for line in table.lines() {
+    let mut line = Vec::new();
+    loop {
+        line.clear();
+        // Bounded line: a row is ~150 bytes; read at most 4 KiB of one (the rest is skipped).
+        let n = read_line_capped(&mut table, &mut line).map_err(|e| unreadable(&e))?;
+        if n == 0 {
+            return Ok(());
+        }
+        let line = String::from_utf8_lossy(&line);
         let f: Vec<&str> = line.split_whitespace().take(8).collect();
         let (Some(l), Some(r), Some(uid)) = (f.get(1).and_then(|a| parse_addr(a)), f.get(2).and_then(|a| parse_addr(a)), f.get(7)) else {
             continue;
@@ -78,26 +103,67 @@ pub(crate) fn same_owner(table: &str, local: SocketAddr, peer: SocketAddr, me: O
         let Ok(uid) = uid.parse::<u32>() else { continue };
         let (l, r) = (key(l), key(r));
         if l == local && r == peer {
-            ours = Some(uid);
+            rows.ours = Some(uid);
         } else if l == peer && r == local {
-            theirs = Some(uid);
+            rows.theirs = Some(uid);
+        }
+        if rows.done() {
+            return Ok(());
         }
     }
-    match (ours.or(me), theirs) {
+}
+
+/// Read one line into `out`, keeping at most 4 KiB of it; returns the bytes consumed (0 at the end).
+fn read_line_capped(r: &mut impl std::io::BufRead, out: &mut Vec<u8>) -> std::io::Result<usize> {
+    const CAP: usize = 4096;
+    let mut total = 0;
+    loop {
+        let buf = r.fill_buf()?;
+        if buf.is_empty() {
+            return Ok(total);
+        }
+        let (take, found) = match buf.iter().position(|b| *b == b'\n') {
+            Some(i) => (i + 1, true),
+            None => (buf.len(), false),
+        };
+        let keep = take.min(CAP.saturating_sub(out.len()));
+        out.extend_from_slice(buf.get(..keep).unwrap_or_default());
+        r.consume(take);
+        total += take;
+        if found {
+            return Ok(total);
+        }
+    }
+}
+
+/// Refuse when the connection's rows show another user. Our uid is our own socket's row, else
+/// `me` (this process's). When the listener isn't listed in a table that was read in full (WSL2,
+/// a proxy, a sandbox) or our uid is unknown, its owner is unknown and the loopback rule decides.
+fn decide(rows: &Rows, peer: SocketAddr, me: Option<u32>) -> Result<(), ZoteroError> {
+    match (rows.ours.or(me), rows.theirs) {
         (Some(a), Some(b)) if a == b => Ok(()),
         (Some(_), Some(b)) => {
-            Err(ZoteroError::Refused(format!("the program on port {} runs as another user (uid {b}), so it isn't your Zotero", peer.1)))
+            Err(ZoteroError::Refused(format!("the program on port {} runs as another user (uid {b}), so it isn't your Zotero", peer.port())))
         }
         _ => {
             log::debug!(
                 "Zotero: couldn't tell which user runs the program on port {} (not in the socket table); trusting the loopback address alone",
-                peer.1
+                peer.port()
             );
             Ok(())
         }
     }
 }
 
+/// From an in-memory `/proc/net/tcp`-format table (tests and callers with a string): see
+/// [`scan_table`] and [`decide`].
+#[cfg_attr(not(test), allow(dead_code))]
+pub(crate) fn same_owner(table: &str, local: SocketAddr, peer: SocketAddr, me: Option<u32>) -> Result<(), ZoteroError> {
+    let mut rows = Rows::default();
+    scan_table(table.as_bytes(), local, peer, &mut rows)?;
+    decide(&rows, peer, me)
+}
+
 /// `0100007F:5A5C` (IPv4) or 32 hex digits and a port (IPv6): each 32-bit word as the kernel
 /// prints it, in this machine's byte order.
 #[cfg_attr(not(any(target_os = "linux", target_os = "android")), allow(dead_code))]
@@ -205,6 +271,51 @@ mod tests {
         assert!(matches!(same_owner(&table, local, peer, None), Err(ZoteroError::Refused(e)) if e.contains("another user")));
     }
 
+    /// A reader that yields `data`, then fails.
+    struct Failing(std::io::Cursor<Vec<u8>>);
+    impl std::io::Read for Failing {
+        fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+            match self.0.read(buf)? {
+                0 => Err(std::io::Error::other("injected")),
+                n => Ok(n),
+            }
+        }
+    }
+
+    /// The real attack: a hostile user floods the table with listener rows so the connection's
+    /// rows sit past any fixed cap. Streaming finds them wherever they are.
+    #[test]
+    fn rows_far_into_a_huge_table_are_still_found() {
+        let (local, peer) = addrs();
+        let filler = row("127.0.0.1:1".parse().unwrap(), "0.0.0.0:0".parse().unwrap(), 1001);
+        let mut table = String::from(HEADER);
+        // About 70 MiB of rows, past the old 64 MiB cap.
+        for _ in 0..(70 * 1024 * 1024 / filler.len()) {
+            table.push_str(&filler);
+        }
+        table.push_str(&row(local, peer, 1000));
+        table.push_str(&row(peer, local, 1001));
+        assert!(table.len() > 64 * 1024 * 1024);
+        assert!(matches!(same_owner(&table, local, peer, None), Err(ZoteroError::Refused(e)) if e.contains("another user")));
+    }
+
+    /// A table that fails mid-read is a refusal, not "listener unknown, trust loopback".
+    #[test]
+    fn a_table_that_fails_to_read_is_refused() {
+        let (local, peer) = addrs();
+        let data = format!("{HEADER}{}", row(local, peer, 1000)).into_bytes();
+        let mut rows = Rows::default();
+        let r = scan_table(std::io::BufReader::new(Failing(std::io::Cursor::new(data))), local, peer, &mut rows);
+        assert!(matches!(r, Err(ZoteroError::Refused(ref e)) if e.contains("socket table")), "{r:?}");
+    }
+
+    #[test]
+    fn an_overlong_line_is_skipped_without_gluing_to_the_next() {
+        let (local, peer) = addrs();
+        let table = format!("{HEADER}{}\n{}{}", "x".repeat(100_000), row(local, peer, 1000), row(peer, local, 1001));
+        assert!(matches!(same_owner(&table, local, peer, None), Err(ZoteroError::Refused(_))));
+    }
+
     #[test]
     fn only_this_computer_is_contacted() {
         for ok in ["127.0.0.1:23116", "127.9.9.9:1", "[::1]:23116", "[::ffff:127.0.0.1]:23116"] {

crates/zotero/src/rich.rs

@@ -42,7 +42,7 @@ fn style_props(base: &CharProps, src: &CharProps) -> CharProps {
 
 /// The body paragraphs of `d` (Zotero's RTF or HTML, read), at most [`MAX_PARAS`], formatted on
 /// top of `base` with the run styling they carry (past [`MAX_RUNS`] runs, text keeps the
-/// formatting before it). Objects are dropped.
+/// formatting before it; each paragraph still needs its first run, so at most [`MAX_PARAS`] more). Objects are dropped.
 fn from_doc(d: &Document, base: &CharProps) -> Vec<Paragraph> {
     let mut out = Vec::new();
     let mut runs = 0usize;