From f398211fd68f2cfa9115425e5800383073b30206 Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 23 Oct 2026 13:11:21 +0000 Subject: [PATCH] OPML nested past 100 levels is refused before it is parsed (#322) The opml crate's Outline reader (hard-xml's derive) recurses once per level with no limit. A 2 MB upload holds tens of thousands of levels, enough to overflow a tokio worker's 2 MiB stack, and a stack overflow aborts the process, not the task. A fetched OPML (up to 50 MB) reached the same parse on every scan, so a publisher could keep the daemon in a crash loop. feed::read_opml counts nesting with quick-xml's streaming reader, which does not recurse, and refuses past 100 before handing the text to the opml crate. Every place that parsed OPML goes through it: the upload, `ipx opml`, a fetched OPML's feeds and its title. The limit also bounds collect_outlines' recursion and the drop of the nested Vec. Co-Authored-By: Claude Opus 5.5 --- crates/feed/src/lib.rs | 47 +++++++++++++++++++++++++++++++++++++++-- crates/ipx/CHANGELOG.md | 1 + crates/ipx/src/main.rs | 2 +- crates/ipx/src/web.rs | 4 ++-- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/crates/feed/src/lib.rs b/crates/feed/src/lib.rs index a1117f6..1e31343 100644 --- a/crates/feed/src/lib.rs +++ b/crates/feed/src/lib.rs @@ -357,10 +357,37 @@ pub fn is_opml(bytes: &[u8]) -> bool { text.contains("` overflowed the +/// stack, which aborts the whole daemon, and again on each restart for a subscribed feed (#322). +pub fn read_opml(text: &str) -> Result { + use quick_xml::events::Event; + let mut reader = quick_xml::Reader::from_str(text); + let mut depth = 0usize; + loop { + match reader.read_event() { + Ok(Event::Start(_)) => { + depth += 1; + if depth > MAX_OPML_DEPTH { + anyhow::bail!("that OPML nests more than {MAX_OPML_DEPTH} levels deep"); + } + } + Ok(Event::End(_)) => depth = depth.saturating_sub(1), + // Malformed XML is the opml crate's to describe. + Ok(Event::Eof) | Err(_) => break, + Ok(_) => {} + } + } + opml::OPML::from_str(text).map_err(|e| anyhow!("{e}")) +} + /// The feeds listed in an OPML document, as (title, xml_url), walking nested folders. pub fn parse_opml(bytes: &[u8]) -> Result> { let text = String::from_utf8_lossy(bytes); - let doc = opml::OPML::from_str(&text).map_err(|e| anyhow!("that does not parse as OPML: {e}"))?; + let doc = read_opml(&text).map_err(|e| anyhow!("that does not parse as OPML: {e}"))?; let mut out = vec![]; collect_outlines(&doc.body.outlines, &mut out); Ok(out) @@ -380,7 +407,7 @@ pub fn collect_outlines(outlines: &[opml::Outline], out: &mut Vec<(String, Strin /// The of an OPML document. pub fn opml_title(bytes: &[u8]) -> Option<String> { let text = String::from_utf8_lossy(bytes); - let doc = opml::OPML::from_str(&text).ok()?; + let doc = read_opml(&text).ok()?; doc.head.and_then(|h| h.title).map(|t| t.trim().to_owned()).filter(|t| !t.is_empty()) } @@ -1809,6 +1836,22 @@ Some **bold** and [a link](https://x.example/).</description></item> assert_eq!((p1.season, p1.episode), (Some(8), Some(1))); } + #[test] + fn deeply_nested_opml_is_refused_before_it_is_parsed() { + let nest = |n: usize| { + format!( + r#"<opml version="2.0"><head><title>T{}{}"#, + r#""#.repeat(n), + "".repeat(n) + ) + }; + assert_eq!(parse_opml(nest(5).as_bytes()).unwrap().len(), 1, "a few folders deep is fine"); + // Deep enough to overflow a 2 MiB stack in the opml crate's reader without the check. + let deep = nest(100_000); + assert!(parse_opml(deep.as_bytes()).unwrap_err().to_string().contains("levels deep")); + assert_eq!(opml_title(deep.as_bytes()), None); + } + #[test] fn opml_is_recognised_and_its_feeds_listed() { let xml = br#"My Subscriptions diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index ba17272..c3dbb17 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An episode that comes as a torrent downloads into a folder of its own, `torrent-` in its podcast's folder. A torrent could name a path anywhere on the server for Delete, the cleanup and playback to act on, and could overwrite other episodes' files of the same name (#318, #331). - A downloaded file opens in the browser only when it is audio, video, an image other than SVG, a PDF or plain text, and is saved otherwise. An SVG or web page a podcast sent ran its script as iPX, signed in as whoever opened it (#319). +- An OPML nested more than 100 levels deep is refused, uploaded or fetched. A deep enough one stopped iPX, and a subscribed one stopped it again each time it restarted (#322). ## [1.4.0] - 2026-10-19 diff --git a/crates/ipx/src/main.rs b/crates/ipx/src/main.rs index c427153..56f4ea7 100644 --- a/crates/ipx/src/main.rs +++ b/crates/ipx/src/main.rs @@ -977,7 +977,7 @@ async fn import(ctx: &Ctx, file: &std::path::Path) -> Result<()> { .into_iter() .find(|u| u.is_admin) .ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add --admin"))?; - let doc = opml::OPML::from_str(&text).map_err(|e| anyhow::anyhow!("{} is not OPML: {e}", file.display()))?; + let doc = feed::read_opml(&text).map_err(|e| anyhow::anyhow!("{} is not OPML: {e}", file.display()))?; let (added, had) = subscribe_opml(ctx, &doc, admin.id).await?; println!("subscribed {} to {added} feed(s); {had} already there", admin.name); Ok(()) diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index c3d06b7..57fd2bc 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -2404,8 +2404,8 @@ async fn import_opml( ) -> Result, ApiError> { // Refused before anything is touched. Nothing reaches the disk either way: an uploaded // file arrives as text, is read here, and is gone when the request ends. - let doc = - opml::OPML::from_str(&body.xml).map_err(|e| ApiError::bad_request(format!("that is not an OPML file: {e}")))?; + let doc = crate::feed::read_opml(&body.xml) + .map_err(|e| ApiError::bad_request(format!("that is not an OPML file: {e}")))?; let (added, already) = crate::subscribe_opml(&state.ctx, &doc, user.id).await?; if added > 0 { scan_soon(&state, None).await;