Harden against hostile-document DoS and non-web link export (#1235)

* Harden against hostile-document DoS and non-web link export

A security pass found several small files that make the app allocate or
compute without bound when opened (GUI, wordcraft-cli, or the MCP server),
and link targets leaving on export without the scheme check the HTML and
Markdown writers already apply. None lets a document run code or read files;
all are denial-of-service or credential-leak-on-click. Each fix bounds an
attacker-controlled size and lands with a regression test, per the
never-crash standard.

- layout: a table repeats its header rows on every page it spills onto, so
  many tall header rows paginate to O(headers x pages) placed items. Cap the
  total header-row repeats per table (lib.rs). 2 KB docx: 1.1 GB -> 124 MB.
- formats/model, html: an HTML table of many <tr> with large rowspan expanded
  to rows x 64 covered cells, and `model::table` pre-allocated a full
  rows x cols grid only to clear it. Cap rows at MAX_ROWS before
  insert_covered, bound its total work, and build the grid template with one
  row. 640 KB html: >1.5 GB -> 214 MB (a plain large table is faster too).
- docx/read/freeform: a custom-geometry Bezier segment took any number of
  points, making de Casteljau flattening quadratic. Cap points per segment.
  3 KB docx: 12 s -> 0.15 s.
- docbin: footnote/header/text-box story tables and the piece table can name
  the same backing text many times. Hold the total characters materialised to
  twice the WordDocument stream size (a ceiling no real file reaches, since
  each char costs >=1 stream byte). 45 KB doc: >1.2 GB -> 14 MB.
- render, pdf, formats/rtf: document pictures were decoded with no dimension
  limit, so a tiny file declaring a huge image could allocate before the size
  check. Decode through image::Limits with a side cap first.
- doc/link, formats/rtf, odt, latex: route exported hyperlink targets through
  is_safe_href (as HTML/Markdown already do), and extend it to reject
  protocol-relative (//host) and UNC (\host) references. Keeps file:,
  javascript:, ms-msdt: and UNC targets from being written into exported RTF,
  ODT, LaTeX and PDF where a reader would follow them (UNC leaks NTLM on
  Windows).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Align picture-decode limits with the bounds already in force

Review of the previous commit: `image::load_from_memory` and
`load_from_memory_with_format` already applied `Limits::default()`, whose
512 MB allocation cap bounded every picture decode. The PDF change had set
`max_alloc = None`, loosening that; restore the default. The render and RTF
side caps (16 384 px) newly rejected wide-but-short pictures that fit in
memory, so render now uses the pixmap's own `u16` side limit (what
`premul_pixmap` enforced after decoding, applied before it instead) and the
RTF re-encode keeps only the explicit allocation cap. Net effect: rejection
moves to header time with no change to the sizes accepted. Also fix a
comment: the header-row repeat cap is per table, not per document.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Jeremy Yao
2026-10-14 18:00:35 -04:00
committed by GitHub
co-authored by Claude Fable 5.1
parent 1190fa8881
commit c3bdcd73b4
11 changed files with 171 additions and 18 deletions
+19 -5
View File
@@ -9,14 +9,23 @@ pub fn is_web_or_mail_link(url: &str) -> bool {
["http", "https", "mailto"].iter().any(|s| scheme.eq_ignore_ascii_case(s))
}
/// Whether a hyperlink may be written as an `href` (or Markdown destination) in an exported file:
/// a web or email link, a bookmark (`#name`) or a relative path. Scripts (`javascript:`,
/// `vbscript:`), `data:`, `file:` and every other scheme are refused, read the way a browser reads
/// a URL: ASCII control characters and spaces are dropped first, so `java\nscript:` is a script.
/// Whether a hyperlink may be written as an `href` (or Markdown destination, RTF `HYPERLINK`
/// field, ODT `xlink:href`, LaTeX `\href`) in an exported file: a web or email link, a bookmark
/// (`#name`) or a relative path. Scripts (`javascript:`, `vbscript:`), `data:`, `file:` and every
/// other scheme are refused, read the way a browser reads a URL: ASCII control characters and
/// spaces are dropped first, so `java\nscript:` is a script. A protocol-relative reference
/// (`//host/…`) or a UNC path (`\\host\…`, or `//host` once backslashes are read as separators)
/// names another machine — following it on Windows leaks the user's NTLM credentials — so it is
/// refused too.
pub fn is_safe_href(url: &str) -> bool {
let cleaned: String = url.chars().filter(|c| !c.is_ascii_control() && *c != ' ').collect();
// `//host` (protocol-relative) and `\\host` (UNC), with `\` read as `/` the way Windows does.
if cleaned.replace('\\', "/").starts_with("//") {
return false;
}
match scheme(url) {
Some(s) => matches!(s.as_str(), "http" | "https" | "mailto"),
None => !url.trim().is_empty(),
None => !cleaned.is_empty(),
}
}
@@ -73,6 +82,11 @@ mod tests {
"data:text/html,<script>alert(1)</script>",
"file:///etc/passwd",
"ms-msdt:/id",
// Protocol-relative and UNC references name another machine (NTLM leak on Windows).
"//attacker.example/share/x",
"\\\\attacker.example\\share\\x",
"/\\attacker.example\\x",
" //attacker.example/x",
"",
] {
assert!(!is_safe_href(bad), "{bad:?}");
+21
View File
@@ -44,6 +44,10 @@ use table::ParaOut;
const MAX_STREAM: u64 = 1 << 30;
/// Most header/footer parts we create (matches the docx reader's cap).
const MAX_PARTS: usize = 50_000;
/// Hard ceiling on characters materialised across every story walk of one document (body,
/// headers, notes, text boxes), whatever the file's size. The working budget is tied to the
/// WordDocument stream (see [`read`]); this caps even a maximal stream.
const MAX_TOTAL_CHARS: u64 = 64 << 20;
/// `sprmCPicLocation`: the fc of a picture in the Data stream.
const C_PIC_LOCATION: u16 = 0x6A03;
/// `sprmPIlvl` / `sprmPIlfo`: the paragraph's list level and list.
@@ -158,6 +162,12 @@ pub fn read_with_password(bytes: &[u8], password: Option<&str>) -> Result<Docume
let hdd_base = fib.ccp.text.saturating_add(fib.ccp.ftn);
let edn_base = hdd_base.saturating_add(fib.ccp.hdd).saturating_add(fib.ccp.atn);
let mut pics = media::Pictures::default();
// Every materialised character comes from at least one byte of the WordDocument stream, so a
// legitimate document's total text (body plus every header, footnote and text box, each read
// once) cannot exceed its stream. A hostile piece or story table that aliases the same bytes
// many times can, so the walk is held to twice the stream's size — generous for real files,
// tight against aliasing — and never past the absolute ceiling.
let budget = std::cell::Cell::new((word.len() as u64).saturating_mul(2).min(MAX_TOTAL_CHARS));
let ctx = WalkCtx {
word: &word,
@@ -172,6 +182,7 @@ pub fn read_with_password(bytes: &[u8], password: Option<&str>) -> Result<Docume
marks: &[],
drawn: &[],
sect_ends: &[],
budget: &budget,
};
// Drawings: text box stories become parts first, then the floating shapes and pictures
@@ -312,6 +323,10 @@ struct WalkCtx<'a> {
/// Where sections end (main-document CPs, sorted): the section mark (0x0C) just before
/// each ends the section's last paragraph, it isn't a page break inside it.
sect_ends: &'a [u32],
/// Characters still allowed across every story walk of this document. Story ranges (headers,
/// footnotes, text boxes) are hostile and can each span a whole subdocument, so thousands of
/// them could otherwise materialise the same text thousands of times; this bounds the total.
budget: &'a std::cell::Cell<u64>,
}
/// State while assembling one paragraph from per-CP formatting.
@@ -385,6 +400,12 @@ fn walk(ctx: &WalkCtx, cp_start: u32, cp_end: u32, pics: &mut media::Pictures) -
continue;
}
};
// Every real character counts against the document-wide budget; once it is spent, stop
// (a hostile file with thousands of story ranges over the same text can't multiply on).
if ctx.budget.get() == 0 {
break;
}
ctx.budget.set(ctx.budget.get() - 1);
// A section mark (0x0C) at a section's end is that section's last paragraph mark.
let section_mark = c == '\u{C}' && ctx.sect_ends.binary_search(&cp.saturating_add(1)).is_ok();
match c {
+38 -1
View File
@@ -7,6 +7,10 @@ use crate::xml::El;
/// Straight pieces a Bézier curve is flattened into.
const CURVE_STEPS: usize = 8;
/// Points read from one path segment. A conformant `a:lnTo`/`a:moveTo` has one, `a:quadBezTo`
/// two and `a:cubicBezTo` three; the cap bounds the de Casteljau control polygon so a segment
/// stuffed with points can't make curve flattening quadratic in the point count.
const MAX_SEG_POINTS: usize = 8;
/// Largest path coordinate kept, in EMUs.
const MAX_EMU: f64 = 1.0e12;
@@ -41,7 +45,7 @@ pub fn cust_geom(sppr: &El, w: f32, h: f32) -> Option<Freeform> {
if cur.len() >= MAX_POINTS || done.len() >= MAX_PATHS {
break;
}
let pts: Vec<[f64; 2]> = seg.children("a:pt").filter_map(pt).collect();
let pts: Vec<[f64; 2]> = seg.children("a:pt").filter_map(pt).take(MAX_SEG_POINTS).collect();
match seg.local() {
"moveTo" => {
if !cur.is_empty() {
@@ -118,3 +122,36 @@ fn bezier(c: &[[f64; 2]], t: f64) -> [f64; 2] {
}
p.first().copied().unwrap_or([0.0, 0.0])
}
#[cfg(test)]
mod tests {
use super::*;
fn sppr(seg: &str) -> crate::xml::El {
let xml = format!(
"<wps:spPr xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\" \
xmlns:wps=\"http://schemas.microsoft.com/office/word/2010/wordprocessingShape\">\
<a:custGeom><a:pathLst><a:path w=\"100\" h=\"100\">\
<a:moveTo><a:pt x=\"0\" y=\"0\"/></a:moveTo>{seg}</a:path></a:pathLst></a:custGeom></wps:spPr>"
);
crate::xml::parse(xml.as_bytes()).expect("parse spPr")
}
#[test]
fn a_bezier_segment_with_many_points_is_bounded() {
// One cubicBezTo stuffed with 50 000 points must not make de Casteljau quadratic: the
// control polygon is capped, so flattening stays a handful of points and returns at once.
let pts = "<a:pt x=\"1\" y=\"1\"/>".repeat(50_000);
let sppr = sppr(&format!("<a:cubicBezTo>{pts}</a:cubicBezTo>"));
let f = cust_geom(&sppr, 100.0, 100.0).expect("a path");
assert!(f.paths.iter().all(|p| p.pts.len() <= 1 + CURVE_STEPS), "{:?}", f.paths.iter().map(|p| p.pts.len()).collect::<Vec<_>>());
}
#[test]
fn a_conformant_cubic_still_flattens_to_a_curve() {
let sppr = sppr("<a:cubicBezTo><a:pt x=\"0\" y=\"100\"/><a:pt x=\"100\" y=\"100\"/><a:pt x=\"100\" y=\"0\"/></a:cubicBezTo>");
let f = cust_geom(&sppr, 100.0, 100.0).expect("a path");
assert_eq!(f.paths.len(), 1);
assert!(f.paths[0].pts.len() >= 2 && f.paths[0].pts.len() <= 1 + CURVE_STEPS);
}
}
+4 -1
View File
@@ -824,7 +824,10 @@ impl<'r> Builder<'r> {
t.rows.push(r);
}
let borderless = !t.bordered;
let mut ft = FTable { rows: t.rows.into_iter().filter(|r| !r.is_empty()).collect(), widths: Vec::new(), borderless };
// Keep at most MAX_ROWS rows (the model drops the rest anyway): bounds the
// O(rows × rowspan) work of `insert_covered` on a table of millions of `<tr>`.
let rows: Vec<_> = t.rows.into_iter().filter(|r| !r.is_empty()).take(wordcraft_doc::table::MAX_ROWS).collect();
let mut ft = FTable { rows, widths: Vec::new(), borderless };
if !ft.rows.is_empty() {
ft.insert_covered();
self.container().push(FBlock::Table(ft));
+5 -1
View File
@@ -317,8 +317,12 @@ impl Writer {
}
if let Some(l) = f.link.as_deref().filter(|l| !l.is_empty()) {
s = match l.strip_prefix('#') {
// An internal bookmark reference, not a URL.
Some(target) => format!("\\hyperlink{{{}}}{{{s}}}", escape_url(target)),
None => format!("\\href{{{}}}{{{s}}}", escape_url(l)),
// A script, `file:`, UNC or other non-web target becomes plain text: a `\href`
// compiles to a PDF `/URI` action the reader would otherwise follow on click.
None if wordcraft_doc::link::is_safe_href(l) => format!("\\href{{{}}}{{{s}}}", escape_url(l)),
None => s,
};
}
s
+39 -1
View File
@@ -265,6 +265,12 @@ impl FTable {
/// After an importer that gives `rowspan` but no placeholder cells (HTML): insert covered
/// cells into the rows below each spanning cell.
pub fn insert_covered(&mut self) {
// Each spanning cell touches up to `rowspan - 1` rows below it; a table of many rows each
// carrying a large rowspan would otherwise be O(rows × rowspan) covered cells, each of
// which becomes a document cell downstream. Cap the total touched so a small file can't
// multiply into a huge table, whatever the rowspans claim.
const MAX_COVERED_WORK: usize = 1 << 17;
let mut work = 0usize;
let nrows = self.rows.len();
for r in 0..nrows {
let mut g = 0usize;
@@ -273,6 +279,10 @@ impl FTable {
let (span, rs, covered) = (c.colspan.clamp(1, 63) as usize, c.rowspan.clamp(1, 1000) as usize, c.covered);
if rs > 1 && !covered {
for k in 1..rs {
work += 1;
if work > MAX_COVERED_WORK {
return;
}
let Some(row) = self.rows.get_mut(r + k) else { break };
// Find the index in that row at grid column `g`.
let mut x = 0usize;
@@ -603,7 +613,10 @@ impl<'a> Builder<'a> {
}
let cols = t.cols().clamp(1, wordcraft_doc::table::MAX_COLS);
let width = self.doc.last_section.text_width().max(72.0);
let mut tb = Table::new(rows.len(), cols, width);
// One template row for the grid widths and props; the real rows are built below. Sizing it
// to `rows.len()` would allocate a full rows × cols cell grid only to clear it, which a
// tall table (many rows, wide from merged cells) turns into a large transient spike.
let mut tb = Table::new(1, cols, width);
if t.borderless {
tb.props.style = None;
}
@@ -1021,6 +1034,21 @@ pub fn image_px(data: &[u8]) -> Option<(u32, u32)> {
image::ImageReader::new(std::io::Cursor::new(data)).with_guessed_format().ok()?.into_dimensions().ok()
}
/// Largest allocation one decoded picture may make when it is re-encoded on export: the `image`
/// crate's default, stated here so the bound doesn't depend on a library default that may change.
const MAX_DECODE_ALLOC: u64 = 512 << 20;
/// Decode an encoded image with a bounded allocation, so a small file that declares huge
/// dimensions can't exhaust memory when a picture is re-encoded on export. `None` when it would
/// be too large or doesn't decode.
pub fn decode_bounded(data: &[u8]) -> Option<image::DynamicImage> {
let mut reader = image::ImageReader::new(std::io::Cursor::new(data)).with_guessed_format().ok()?;
let mut limits = image::Limits::default();
limits.max_alloc = Some(MAX_DECODE_ALLOC);
reader.limits(limits);
reader.decode().ok()
}
/// A picture from bytes with an optional display size (points); the natural size (96 dpi) is
/// used otherwise, shrunk to fit a 6.5" column.
pub fn make_img(data: impl Into<Arc<Vec<u8>>>, w: Option<f32>, h: Option<f32>, alt: &str) -> Option<Img> {
@@ -1225,6 +1253,16 @@ pub fn parse_color(s: &str) -> Option<Rgb> {
mod tests {
use super::*;
#[test]
fn insert_covered_is_bounded_on_hostile_rowspan() {
// Many rows each carrying the maximum rowspan would be O(rows × rowspan) covered cells;
// the work cap must keep insertion bounded (and quick) instead of exhausting memory.
let mut t =
FTable { rows: (0..20_000).map(|_| vec![Cell { rowspan: 1000, ..Default::default() }]).collect(), widths: Vec::new(), borderless: false };
t.insert_covered();
assert!(t.rows.iter().all(|r| r.len() <= 64), "a row grew past the column cap");
}
#[test]
fn base64_round_trip() {
for data in [&b""[..], b"f", b"fo", b"foo", b"foob", b"fooba", b"foobar", &[0, 255, 128, 7]] {
+3 -1
View File
@@ -357,7 +357,9 @@ impl Writer {
flush(&mut body, &mut spaces, prev_space);
prev_space = true;
}
let link = f.link.as_ref();
// A script, `file:`, UNC or other non-web target is written as plain text, as
// the HTML and Markdown exporters do (LibreOffice follows `file:`/UNC hrefs).
let link = f.link.as_deref().filter(|l| wordcraft_doc::link::is_safe_href(l));
if let Some(l) = link {
out.push_str(&format!("<text:a xlink:type=\"simple\" xlink:href=\"{}\">", x(l)));
}
+7 -3
View File
@@ -173,7 +173,10 @@ impl Writer {
ctl.push_str(&format!("\\chcbpat{i}"));
}
let body = if ctl.is_empty() { esc(t) } else { format!("{{{ctl} {}}}", esc(t)) };
match &f.link {
// A script, `file:`, UNC or other non-web target is written as plain text, as the HTML and
// Markdown exporters do: Word follows a HYPERLINK field's target on click, and a UNC one
// leaks the reader's NTLM credentials.
match f.link.as_deref().filter(|l| wordcraft_doc::link::is_safe_href(l)) {
Some(l) => {
out.push_str(&format!("{{\\field{{\\*\\fldinst{{HYPERLINK \"{}\"}}}}{{\\fldrslt{{{}}}}}}}", esc(&l.replace('"', "%22")), body));
}
@@ -186,8 +189,9 @@ impl Writer {
"png" => (img.data.to_vec(), "\\pngblip"),
"jpeg" | "jpg" => (img.data.to_vec(), "\\jpegblip"),
_ => {
// Re-encode other formats as PNG.
let Ok(dec) = image::load_from_memory(&img.data) else { return };
// Re-encode other formats as PNG, bounding the decode so a tiny file declaring
// huge dimensions can't exhaust memory on export.
let Some(dec) = model::decode_bounded(&img.data) else { return };
let mut buf = Vec::new();
if dec.write_to(&mut std::io::Cursor::new(&mut buf), image::ImageFormat::Png).is_err() {
return;
+13 -2
View File
@@ -3456,6 +3456,13 @@ fn place_table(ctx: &mut Ctx, pb: &mut PageBuilder, t: &wordcraft_doc::Table, bl
// A table in the text flow starts below the floating objects in its way.
below_floats(pb, pb.col_x() + tl.x, pb.col_x() + tl.x + tl.width);
let header_rows: Vec<usize> = (0..t.rows.len()).take_while(|r| t.rows.get(*r).is_some_and(|row| row.props.header)).collect();
// Header rows are re-placed on every page a table spills onto, so a table with many header
// rows that each spill onto their own page is O(header rows × pages) placed items — a few KB
// of document can otherwise fill memory. Once this many header rows have been repeated across
// the whole table, later pages carry none; pagination still finishes. Far above any real
// table (e.g. 3 header rows over 30 000 pages).
const MAX_HEADER_REPEATS: usize = 200_000;
let mut header_repeats = 0usize;
let place = |pb: &mut PageBuilder, row: &table::RowLayout| {
let (x, y) = (pb.col_x() + tl.x, pb.y);
if let Some(pg) = pb.page() {
@@ -3497,11 +3504,15 @@ fn place_table(ctx: &mut Ctx, pb: &mut PageBuilder, t: &wordcraft_doc::Table, bl
None => {}
}
pb.advance(block, body_top);
// Repeat header rows.
if !header_rows.contains(&ri) {
// Repeat header rows, up to this table's cap.
if !header_rows.contains(&ri) && header_repeats < MAX_HEADER_REPEATS {
for hr in &header_rows {
if header_repeats >= MAX_HEADER_REPEATS {
break;
}
if let Some(h) = tl.rows.get(*hr) {
place(pb, h);
header_repeats += 1;
}
}
}
+9 -1
View File
@@ -1641,7 +1641,15 @@ fn path_from(b: &kurbo::BezPath) -> Option<Path> {
/// can't fail the whole export).
fn load_image(data: &Arc<Vec<u8>>) -> Option<Image> {
let fmt = image::guess_format(data).ok()?;
let dec = image::load_from_memory_with_format(data, fmt).ok()?;
// The side check below used to run only after a full decode; applying it as a decode limit
// rejects a small file that declares a huge image at header time instead. The default
// allocation cap (512 MB, what `load_from_memory_with_format` applied) stays in force.
let mut reader = image::ImageReader::new(std::io::Cursor::new(data.as_slice())).with_guessed_format().ok()?;
let mut limits = image::Limits::default();
limits.max_image_width = Some(MAX_IMAGE_SIDE);
limits.max_image_height = Some(MAX_IMAGE_SIDE);
reader.limits(limits);
let dec = reader.decode().ok()?;
let (w, h) = (dec.width(), dec.height());
if w == 0 || h == 0 || w > MAX_IMAGE_SIDE || h > MAX_IMAGE_SIDE {
return None;
+13 -2
View File
@@ -167,9 +167,20 @@ fn vectors() -> &'static Mutex<MediaCache<Option<Arc<Vector>>>> {
C.get_or_init(|| Mutex::new(MediaCache { map: HashMap::new(), bytes: 0 }))
}
/// Decode an encoded image to a premultiplied pixmap.
/// Largest side (pixels) a document picture may decode to: what a `Pixmap` can hold (`u16`
/// sides, which [`premul_pixmap`] enforces after decoding). Applying it as a decode limit rejects
/// a small file that declares a bigger image at header time, before anything is allocated.
const MAX_DECODE_SIDE: u32 = u16::MAX as u32;
/// Decode an encoded image to a premultiplied pixmap. The dimension limit above and the default
/// allocation cap (512 MB, what `load_from_memory` applied) bound the decode during paint.
pub fn decode_pixmap(bytes: &[u8]) -> Option<Pixmap> {
let img = image::load_from_memory(bytes).ok()?.to_rgba8();
let mut reader = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?;
let mut limits = image::Limits::default();
limits.max_image_width = Some(MAX_DECODE_SIDE);
limits.max_image_height = Some(MAX_DECODE_SIDE);
reader.limits(limits);
let img = reader.decode().ok()?.to_rgba8();
let (w, h) = img.dimensions();
premul_pixmap(w, h, img.as_raw())
}