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
<outline> 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 <noreply@anthropic.com>
This commit is contained in:
2026-10-23 13:11:21 +00:00
parent 708b69c083
commit f398211fd6
4 changed files with 49 additions and 5 deletions

View File

@@ -357,10 +357,37 @@ pub fn is_opml(bytes: &[u8]) -> bool {
text.contains("<opml") text.contains("<opml")
} }
/// How deep an OPML document's elements may nest. Real ones are a folder or two deep.
const MAX_OPML_DEPTH: usize = 100;
/// Parses an OPML document, refusing one nested deeper than `MAX_OPML_DEPTH`. The opml crate's
/// reader recurses once per level with no limit, and a few MB of `<outline>` 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<opml::OPML> {
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. /// The feeds listed in an OPML document, as (title, xml_url), walking nested folders.
pub fn parse_opml(bytes: &[u8]) -> Result<Vec<(String, String)>> { pub fn parse_opml(bytes: &[u8]) -> Result<Vec<(String, String)>> {
let text = String::from_utf8_lossy(bytes); 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![]; let mut out = vec![];
collect_outlines(&doc.body.outlines, &mut out); collect_outlines(&doc.body.outlines, &mut out);
Ok(out) Ok(out)
@@ -380,7 +407,7 @@ pub fn collect_outlines(outlines: &[opml::Outline], out: &mut Vec<(String, Strin
/// The <head><title> of an OPML document. /// The <head><title> of an OPML document.
pub fn opml_title(bytes: &[u8]) -> Option<String> { pub fn opml_title(bytes: &[u8]) -> Option<String> {
let text = String::from_utf8_lossy(bytes); 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()) 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))); 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</title></head><body>{}<outline text="f" xmlUrl="https://e.com/f"/>{}</body></opml>"#,
r#"<outline text="a">"#.repeat(n),
"</outline>".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] #[test]
fn opml_is_recognised_and_its_feeds_listed() { fn opml_is_recognised_and_its_feeds_listed() {
let xml = br#"<opml version="2.0"><head><title>My Subscriptions</title></head><body> let xml = br#"<opml version="2.0"><head><title>My Subscriptions</title></head><body>

View File

@@ -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-<id>` 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). - An episode that comes as a torrent downloads into a folder of its own, `torrent-<id>` 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). - 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 ## [1.4.0] - 2026-10-19

View File

@@ -977,7 +977,7 @@ async fn import(ctx: &Ctx, file: &std::path::Path) -> Result<()> {
.into_iter() .into_iter()
.find(|u| u.is_admin) .find(|u| u.is_admin)
.ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add <name> --admin"))?; .ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add <name> --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?; let (added, had) = subscribe_opml(ctx, &doc, admin.id).await?;
println!("subscribed {} to {added} feed(s); {had} already there", admin.name); println!("subscribed {} to {added} feed(s); {had} already there", admin.name);
Ok(()) Ok(())

View File

@@ -2404,8 +2404,8 @@ async fn import_opml(
) -> Result<Json<serde_json::Value>, ApiError> { ) -> Result<Json<serde_json::Value>, ApiError> {
// Refused before anything is touched. Nothing reaches the disk either way: an uploaded // 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. // file arrives as text, is read here, and is gone when the request ends.
let doc = let doc = crate::feed::read_opml(&body.xml)
opml::OPML::from_str(&body.xml).map_err(|e| ApiError::bad_request(format!("that is not an OPML file: {e}")))?; .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?; let (added, already) = crate::subscribe_opml(&state.ctx, &doc, user.id).await?;
if added > 0 { if added > 0 {
scan_soon(&state, None).await; scan_soon(&state, None).await;