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:
@@ -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>
|
||||||
|
|||||||
@@ -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
|
||||||
|
|
||||||
|
|||||||
@@ -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(())
|
||||||
|
|||||||
@@ -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;
|
||||||
|
|||||||
Reference in New Issue
Block a user