links: a linked file is read under a cap, not whole (#1596)

Update, Relink and Relink Folder all reload a linked graphic through `reload`,
which read the file with an unbounded `read`. A document whose link points at a
multi-GB file exhausted memory on Update. How large a linked file is belongs to
whoever wrote the document, not to whoever opens it: the path comes out of the
document.

`reload` now reads through `read_capped` at the limit packaging already applies
to a linked file (`MAX_LINKED_FILE_BYTES`, 256 MB), and a file over it is
refused by name instead of read. `reload_capped` takes the cap so a test can
show both sides without a 256 MB fixture.

Not changed: `file.place` reads a path the same unbounded way, but there the
user picked the file, where a link's path can come from a document someone else
wrote. Whether that read wants the same cap is a call for a maintainer, so it
is reported rather than changed.

Found in the audit of #1596 (DC-15).
This commit is contained in:
Hamza
2026-10-24 16:07:21 +05:00
parent c313c53497
commit 332b0d9f5c
+40 -1
View File
@@ -396,8 +396,23 @@ fn reimport(d: &mut Document, sid: StoryId, path: &str, bytes: &[u8], before: Fi
#[cfg(not(target_arch = "wasm32"))]
fn reload(path: impl AsRef<Path>) -> Result<(Vec<u8>, (u32, u32))> {
reload_capped(path, super::interchange::MAX_LINKED_FILE_BYTES)
}
/// A linked graphic read again from disk, and its pixel size, reading at most `cap` bytes.
///
/// How large a linked file is belongs to whoever wrote the document, not to whoever opens it: the
/// path comes out of the document, and Update, Relink and Relink Folder all follow it. An
/// unbounded `read` of a link pointing at a multi-GB file exhausted memory (#1596). The cap is
/// the one packaging already applies to a linked file, and a file over it is refused by name
/// rather than read.
#[cfg(not(target_arch = "wasm32"))]
fn reload_capped(path: impl AsRef<Path>, cap: u64) -> Result<(Vec<u8>, (u32, u32))> {
let path = path.as_ref();
let b = crate::io::read_file(path).map_err(EngineError::Other)?;
let b = crate::io::read_capped(path, cap).map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?;
if b.len() as u64 > cap {
return Err(bad("links", format!("{}: the linked file is larger than {} MB", path.display(), cap / (1024 * 1024))));
}
if designcraft_render::is_pdf(&b) {
designcraft_render::check_pdf_input_size(b.len()).map_err(|e| bad("links", format!("{}: {e}", path.display())))?;
}
@@ -584,6 +599,30 @@ mod tests {
designcraft_render::Rendered { width: w, height: h, pixels: vec![200u8; (w * h * 4) as usize] }.to_png()
}
/// A linked file's size is the document's choice, so a link is read under a cap (#1596).
#[test]
fn a_linked_file_over_the_cap_is_refused_instead_of_read() {
let dir = std::env::temp_dir().join(format!("dc-link-cap-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("big.png");
let bytes = png(40, 40);
std::fs::write(&path, &bytes).unwrap();
// Under the cap the file is read and measured as before.
let (read, px) = reload_capped(&path, bytes.len() as u64).expect("a file within the cap is read");
assert_eq!((read.len(), px), (bytes.len(), (40, 40)));
// Over it the file is refused by name, and the error says what the limit is.
let err = reload_capped(&path, (bytes.len() - 1) as u64).expect_err("a file over the cap is refused");
let msg = err.to_string();
assert!(msg.contains("big.png") && msg.contains("larger than"), "the error names the file and the limit: {msg}");
// The real cap is the one packaging uses, so an ordinary graphic still loads.
assert!(reload(&path).is_ok(), "a normal linked graphic is unaffected");
let _ = std::fs::remove_dir_all(&dir);
}
fn pdf(sizes: &[(u32, u32)]) -> Vec<u8> {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": sizes.len(), "facingPages": false})).unwrap();