security: budget the rendering pipeline against DoS geometry (CWE-400/407/789) (#1849)

* security: budget the rendering pipeline against DoS geometry (CWE-400/407/789)

Several render paths let crafted drawing data spin the rendering thread or
allocate without bound:

- Hatch dash sweeping reset its per-span step counter and used emitted geometry
  as its only limit, so gap-only families (which emit nothing) could multiply
  CPU work. Skip families that cannot draw and charge every dash step against a
  shared per-hatch work budget (row 10).
- Hatch polygon deduplication compared loops with an O(n²) cyclic match and
  every loop against every kept one. Charge vertex comparisons against a shared
  budget and stop deduplicating when spent (row 12).
- Table coverage marked every merge rectangle, so overlapping merges re-marked
  covered cells quadratically. Bound marking to a small multiple of the cell
  count (row 11).
- Broken-table part layout expanded sparse rows to full width before checking
  whether detailed layout is allowed. Only expand when within the cell limit;
  otherwise leave cells sparse and let layout draw an outline (row 17).
- Dimension text substitution copied the base for every `<>` placeholder with no
  size bound. Estimate the expansion and cap it at a fixed text budget (row 32).
- Linetype range generation cycled all-gap patterns forever (nothing emitted, so
  the emit limit never tripped). Return continuous for all-gap patterns and
  bound visited pattern elements (row 38).
- Viewport contents built with a fresh expansion counter, so a layout could far
  exceed the documented block-expansion limit. Share one budget across the whole
  build, seeded with what the enclosing list already spent (row 22).

New tests for the all-gap linetype path; all render tests pass, clippy clean.

Addresses findings rows 10, 11, 12, 17, 22, 32, 38.

* fix: preserve bounded render fallbacks and text output

---------

Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com>
This commit is contained in:
Dylan M. Taylor
2026-10-22 17:08:25 -04:00
committed by GitHub
co-authored by skylartaylor
parent 7c72ec45a9
commit 2dbc7699a0
5 changed files with 213 additions and 29 deletions
+70 -16
View File
@@ -53,17 +53,30 @@ pub struct TableGeom {
/// Which cells are covered by a merge (and so not drawn on their own).
pub fn table_covered(t: &Table) -> Vec<Vec<bool>> {
coverage(t).unwrap_or_else(|partial| partial)
}
// No partial coverage may reach owner assignment: it would revisit unbounded merge rectangles.
fn coverage(t: &Table) -> Result<Vec<Vec<bool>>, Vec<Vec<bool>>> {
let rows = t.row_heights.len().min(10_000);
let cols = t.col_widths.len().min(10_000);
if rows.saturating_mul(cols) > MAX_LAYOUT_CELLS {
return vec![Vec::new(); rows];
return Err(vec![Vec::new(); rows]);
}
let mut cov = vec![vec![false; cols]; rows];
// Coverage marking visits each merge's whole rectangle. Overlapping merges re-mark cells that
// are already covered, so bound the total marking steps to a small multiple of the cell count
// rather than letting overlapping declarations cause quadratic work (CWE-407).
let mut budget: u64 = (rows as u64).saturating_mul(cols as u64).saturating_mul(4).saturating_add(1024);
for (r, row) in t.cells.iter().enumerate().take(rows) {
for (c, cell) in row.iter().enumerate().take(cols) {
if let Some((rs, cs)) = cell.merged {
for rr in r..(r + rs.max(1) as usize).min(rows) {
for cc in c..(c + cs.max(1) as usize).min(cols) {
budget = budget.saturating_sub(1);
if budget == 0 {
return Err(cov);
}
if (rr, cc) != (r, c)
&& let Some(x) = cov.get_mut(rr).and_then(|v| v.get_mut(cc))
{
@@ -74,7 +87,7 @@ pub fn table_covered(t: &Table) -> Vec<Vec<bool>> {
}
}
}
cov
Ok(cov)
}
/// The table style a table uses (Standard defaults when missing).
@@ -134,19 +147,25 @@ pub fn table_geometry(d: &Drawing, t: &Table) -> TableGeom {
/// One part of a broken table as a table of its own: its rows (with their cell styles) at
/// `origin`; merges are cut at the part's edge.
fn part_table(t: &Table, rows: &[usize], origin: Vec2) -> Table {
let cols = t.col_widths.len();
let mut cells: Vec<Vec<cadcraft_doc::TableCell>> = rows.iter().map(|r| t.cells.get(*r).cloned().unwrap_or_default()).collect();
for (i, row) in cells.iter_mut().enumerate() {
row.resize_with(cols, Default::default);
for cell in row.iter_mut() {
if let Some((rs, cs)) = cell.merged {
// Rows of the merge that follow in this part.
let start = rows.get(i).copied().unwrap_or(0);
let mut n = 1u32;
while n < rs && rows.get(i + n as usize).is_some_and(|r| *r == start + n as usize) {
n += 1;
let cols = t.col_widths.len().min(10_000);
let mut cells: Vec<Vec<cadcraft_doc::TableCell>> =
rows.iter().take(10_000).map(|r| t.cells.get(*r).map(|row| row.iter().take(cols).cloned().collect()).unwrap_or_default()).collect();
// Only expand sparse rows to the full column count when detailed layout is actually permitted.
// Otherwise `layout` renders an outline, so materialising millions of empty cells here would be
// wasted work an attacker can trigger with a compact dimension array (CWE-789).
if rows.len().min(10_000).saturating_mul(cols) <= MAX_LAYOUT_CELLS {
for (i, row) in cells.iter_mut().enumerate() {
row.resize_with(cols, Default::default);
for cell in row.iter_mut() {
if let Some((rs, cs)) = cell.merged {
// Rows of the merge that follow in this part.
let start = rows.get(i).copied().unwrap_or(0);
let mut n = 1u32;
while n < rs && rows.get(i + n as usize).is_some_and(|r| *r == start + n as usize) {
n += 1;
}
cell.merged = (n > 1 || cs > 1).then_some((n, cs));
}
cell.merged = (n > 1 || cs > 1).then_some((n, cs));
}
}
}
@@ -180,7 +199,8 @@ fn layout(d: &Drawing, t: &Table) -> TableGeom {
if rows == 0 || cols == 0 {
return g;
}
if rows.saturating_mul(cols) > MAX_LAYOUT_CELLS {
let cov = coverage(t);
if rows.saturating_mul(cols) > MAX_LAYOUT_CELLS || cov.is_err() {
let (w, h) = (x_at(cols), y_at(rows));
let corners = [p(0.0, 0.0), p(w, 0.0), p(w, h), p(0.0, h), p(0.0, 0.0)];
for s in corners.windows(2) {
@@ -209,7 +229,7 @@ fn layout(d: &Drawing, t: &Table) -> TableGeom {
i
};
// Anchors and the anchor owning each grid cell.
let cov = table_covered(t);
let cov = cov.unwrap_or_default();
let mut anchors: Vec<Anchor> = Vec::new();
let mut owner = vec![usize::MAX; rows * cols];
for r in 0..rows {
@@ -369,3 +389,37 @@ pub(crate) fn draw(b: &mut Builder, ctx: &Ctx, t: &Table, rgb: Ink, lw: f32) {
}
}
}
#[cfg(test)]
mod budget_tests {
use super::*;
use cadcraft_doc::TableCell;
#[test]
fn overlapping_merges_use_outline_fallback() {
let t = Table {
row_heights: vec![1.0; 40],
col_widths: vec![1.0; 40],
cells: vec![vec![TableCell { merged: Some((40, 40)), ..Default::default() }; 40]; 40],
..Default::default()
};
assert!(coverage(&t).is_err());
let g = layout(&Drawing::default(), &t);
assert_eq!(g.lines.len(), 4);
assert!(g.fills.is_empty() && g.texts.is_empty());
}
#[test]
fn part_clips_merges_using_displayed_column_count() {
let t = Table {
row_heights: vec![1.0; 102],
col_widths: vec![1.0; 10_001],
cells: vec![vec![TableCell { merged: Some((100, 1)), ..Default::default() }]],
..Default::default()
};
let mut rows: Vec<_> = (0..99).collect();
rows.push(101);
let part = part_table(&t, &rows, Vec2::ZERO);
assert_eq!(part.cells[0][0].merged, Some((99, 1)));
}
}