diff --git a/crates/zotero/src/peer.rs b/crates/zotero/src/peer.rs index 07df98e79..09abd8de4 100644 --- a/crates/zotero/src/peer.rs +++ b/crates/zotero/src/peer.rs @@ -4,9 +4,10 @@ //! - **Every system:** WordCraft only connects to this computer (a loopback address). //! - **Linux only:** the program listening there must not run as another user, from the //! kernel's socket table (`/proc/net/tcp`): another account on a shared machine that took the -//! port while Zotero wasn't running is refused. A listener the table doesn't show (WSL2 -//! mirrored networking with Zotero on Windows, a local proxy, a sandbox) leaves only the -//! loopback rule. +//! port while Zotero wasn't running is refused. A listener the tables don't show (WSL2 +//! mirrored networking with Zotero on Windows, a local proxy) leaves only the loopback rule, +//! but only when the tables were read in full: a read error, or an open error other than +//! "not found" (e.g. a sandbox profile that denies `/proc/net/tcp`), refuses. //! //! On Windows, macOS and the BSDs there is **no same-user check**: the loopback rule is the //! only one, so any local account's listener on the port is taken for Zotero. Those systems have @@ -35,23 +36,13 @@ 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(); - 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); - } - } - } + let rows = scan_tables(["/proc/net/tcp", "/proc/net/tcp6"], |p| std::fs::File::open(p), local, peer)?; // 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,43 +52,134 @@ 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. +/// Scan the tables named by `paths` (opened by `open`) in order, stopping once both rows are +/// found. A table that doesn't exist (no IPv6) is skipped; any other open or read error refuses. #[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> { +fn scan_tables( + paths: [&str; 2], + open: impl Fn(&str) -> std::io::Result, + local: SocketAddr, + peer: SocketAddr, +) -> Result { + let mut rows = Rows::default(); + for path in paths { + match open(path) { + Ok(f) => scan_table(std::io::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; + } + } + Ok(rows) +} + +/// 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 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 { + 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 mut f = line.split_whitespace(); + let (Some(l), Some(r), Some(uid)) = (f.nth(1).and_then(parse_addr), f.next().and_then(parse_addr), f.nth(4)) else { continue; }; 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 = match r.fill_buf() { + Ok(b) => b, + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => return Err(e), + }; + 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) 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))] @@ -167,14 +249,14 @@ mod tests { assert_eq!(same_owner(&junk, local, peer, None), Ok(())); } - /// WSL2 mirrored networking (Zotero on Windows), a local proxy or a sandbox: the listener + /// WSL2 mirrored networking (Zotero on Windows) or a local proxy: the listener /// isn't in our table, so the loopback rule decides, as on other systems. #[test] fn an_unlisted_listener_falls_back_to_the_loopback_rule() { let (local, peer) = addrs(); let missing = format!("{HEADER}{}", row(local, peer, 1000)); assert_eq!(same_owner(&missing, local, peer, None), Ok(())); - // The table couldn't be read. + // An empty table: the listener isn't listed. assert_eq!(same_owner("", local, peer, None), Ok(())); assert_eq!(same_owner("", local, peer, Some(1000)), Ok(())); } @@ -205,6 +287,105 @@ 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), + } + } + } + + /// Repeats `unit` for `left` bytes, then ends: a huge table without holding it in memory. + struct Table { + parts: Vec<(Vec, usize)>, + part: usize, + at: usize, + } + impl std::io::Read for Table { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let Some((unit, times)) = self.parts.get(self.part) else { return Ok(0) }; + if self.at >= unit.len() * times { + self.part += 1; + self.at = 0; + return self.read(buf); + } + let n = buf.len().min(unit.len() - self.at % unit.len()); + let off = self.at % unit.len(); + buf[..n].copy_from_slice(&unit[off..off + n]); + self.at += 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. The real read path (`scan_tables`, which `owner_check` + /// calls on `/proc/net/tcp*`) 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); + // More than 64 MiB of rows (the old cap), then the connection's two rows. + let times = 70 * 1024 * 1024 / filler.len(); + assert!(filler.len() * times > 64 * 1024 * 1024); + let tail = format!("{}{}", row(local, peer, 1000), row(peer, local, 1001)).into_bytes(); + let open = |p: &str| match p { + "t4" => { + Ok(Table { parts: vec![(HEADER.as_bytes().to_vec(), 1), (filler.clone().into_bytes(), times), (tail.clone(), 1)], part: 0, at: 0 }) + } + _ => Err(std::io::Error::from(std::io::ErrorKind::NotFound)), + }; + let rows = scan_tables(["t4", "t6"], open, local, peer).unwrap(); + assert_eq!((rows.ours, rows.theirs), (Some(1000), Some(1001))); + assert!(matches!(decide(&rows, peer, None), Err(ZoteroError::Refused(e)) if e.contains("another user"))); + } + + /// A signal interrupting a read is retried, not taken for an unreadable table. + #[test] + fn an_interrupted_read_is_retried() { + struct Eintr(bool, std::io::Cursor>); + impl std::io::Read for Eintr { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if !std::mem::replace(&mut self.0, true) { + return Err(std::io::ErrorKind::Interrupted.into()); + } + self.1.read(buf) + } + } + let (local, peer) = addrs(); + let data = format!("{HEADER}{}{}", row(local, peer, 7), row(peer, local, 7)).into_bytes(); + let open = |_: &str| Ok(Eintr(false, std::io::Cursor::new(data.clone()))); + let rows = scan_tables(["a", "b"], open, local, peer).unwrap(); + assert_eq!((rows.ours, rows.theirs), (Some(7), Some(7))); + } + + /// A sandbox that denies the table (not "not found") refuses. + #[test] + fn a_table_that_cannot_be_opened_is_refused() { + let (local, peer) = addrs(); + let open = |_: &str| -> std::io::Result<&[u8]> { Err(std::io::ErrorKind::PermissionDenied.into()) }; + assert!(matches!(scan_tables(["a", "b"], open, local, peer), Err(ZoteroError::Refused(_)))); + } + + /// 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; diff --git a/docs/zotero.md b/docs/zotero.md index cf5813494..4e96520ad 100644 --- a/docs/zotero.md +++ b/docs/zotero.md @@ -74,8 +74,10 @@ isn't another user's. On Windows and macOS (and the BSDs) the loopback rule is t program listening on the port runs as the same user as WordCraft, before sending anything. If another account on a shared machine took the port while Zotero wasn't running, WordCraft refuses with an error and the document isn't touched. When the table doesn't show the - listener (WSL2 mirrored networking with Zotero running on Windows, a local proxy, a sandbox) - or can't be read, its owner is unknown and only the loopback rule applies, as below. + listener (WSL2 mirrored networking with Zotero running on Windows, a local proxy), its owner + is unknown and only the loopback rule applies, as below. That holds only when the tables were + read in full: a read error, or an open error other than "not found", **refuses**. Sandboxes + that deny `/proc/net/tcp` (some AppArmor or Snap profiles, for example) are therefore refused. - Windows, macOS and the BSDs: **no same-user check.** They have no such table to read without OS calls WordCraft doesn't make (it uses no `unsafe` code), so only the loopback rule applies there (and on Linux when the listener isn't in the table). Remaining risk: on a