diff --git a/crates/zotero/src/peer.rs b/crates/zotero/src/peer.rs index 07df98e79..68e218536 100644 --- a/crates/zotero/src/peer.rs +++ b/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) -> Result<(), ZoteroError> { +/// The uids found for the two rows of one connection. +#[derive(Default)] +struct Rows { + ours: Option, + theirs: Option, +} + +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::() 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) -> std::io::Result { + 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) -> 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) -> 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>); + impl std::io::Read for Failing { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + 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"] { diff --git a/crates/zotero/src/rich.rs b/crates/zotero/src/rich.rs index 7ff3377f1..705a7e15e 100644 --- a/crates/zotero/src/rich.rs +++ b/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 { let mut out = Vec::new(); let mut runs = 0usize;