Security: bounded flattening, plug-in budgets and caches, loaded textures, Revolve paint copies, color book names and ICC names

CodeRabbit findings #4, #34, #46, #51, #77, #101, #170, #171, #188:

- `vectorcraft_geom::flatten`: kurbo's flatten with each curve's tolerance raised so it makes
  at most ~4096 segments (non-finite curves become a line); the canvas outline and
  variable-width strokes use it.
- Plug-ins: a heavy filter's budget is the host's `Limits::heavy_fuel`/`heavy_wall_time_ms`,
  never more; a start function runs on one fuel slice; the effect cache holds at most ~1M
  anchors of inputs and results, and doesn't keep a result past a quarter of that.
- Loaded textures are cached by their PNG (base64 spacing doesn't matter), and live textures
  hold at most 512 MB together.
- Revolve draws the source path when the faces' paint copies would pass 256 MB.
- Color books: names with prefix and suffix take at most 64 MB.
- ICC: localized-name (`mluc`) records over 1 MB together are refused before moxcms copies them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Dylan M. Taylor
2026-10-24 17:19:54 +07:00
committed by bflatastic
co-authored by Claude Opus 5.5
parent a608838ba2
commit e2a870e602
10 changed files with 257 additions and 28 deletions
+34
View File
@@ -95,6 +95,9 @@ impl IccProfile {
}
pub fn from_bytes(name: Option<String>, bytes: &[u8]) -> Result<Self, CmsError> {
if !localized_text_fits(bytes) {
return Err(CmsError::BadProfile(format!("its localized names take more than {} KB", MAX_LOCALIZED_TEXT >> 10)));
}
let p = ColorProfile::new_from_slice(bytes).map_err(|e| CmsError::BadProfile(format!("{e:?}")))?;
Ok(Self { source: bytes.to_vec(), ..Self::from_profile(name, p)? })
}
@@ -393,3 +396,34 @@ pub(super) fn gray_profile(name: &str) -> ColorProfile {
p.gray_trc = ColorProfile::new_srgb().red_trc;
labelled(p, name)
}
/// Most bytes of UTF-16 the localized names (`mluc` tags) of a profile may take together.
const MAX_LOCALIZED_TEXT: usize = 1 << 20;
/// Whether the localized-name records of the `mluc` tags in ICC `bytes` take at most
/// [`MAX_LOCALIZED_TEXT`] together. moxcms copies each record into a string of its own, and records
/// may all point at the same bytes, so a profile under a megabyte could make it allocate
/// gigabytes. A table it can't read is left to moxcms to refuse.
pub(crate) fn localized_text_fits(bytes: &[u8]) -> bool {
let be32 = |b: &[u8], at: usize| b.get(at..at.checked_add(4)?).map(|v| u32::from_be_bytes([v[0], v[1], v[2], v[3]]) as usize);
let Some(tags) = be32(bytes, 128) else { return true };
let mut total = 0usize;
for i in 0..tags {
let Some(entry) = i.checked_mul(12).and_then(|e| e.checked_add(132)) else { return false };
let (Some(offset), Some(size)) = (be32(bytes, entry + 4), be32(bytes, entry + 8)) else { break };
let Some(tag) = offset.checked_add(size).and_then(|end| bytes.get(offset..end)) else { continue };
if !tag.starts_with(b"mluc") {
continue;
}
let records = be32(tag, 8).unwrap_or(0);
for r in 0..records {
// Each record's header: language, country, length, offset.
let Some(length) = r.checked_mul(12).and_then(|h| be32(tag, h + 16 + 4)) else { break };
total = total.saturating_add(length);
if total > MAX_LOCALIZED_TEXT {
return false;
}
}
}
true
}
+28
View File
@@ -152,3 +152,31 @@ fn cmyk_profiles_have_the_tags_of_an_output_profile() {
assert!(p.lut_a_to_b_saturation.is_some() && p.lut_b_to_a_saturation.is_some() && p.gamut.is_some(), "{name}");
}
}
/// A profile whose localized-name records all point at one long string is refused before moxcms
/// copies each of them; a few short names pass.
#[test]
fn aliased_localized_names_are_refused() {
let profile = |records: u32, length: u32| {
let mut b = vec![0u8; 128];
b.extend(1u32.to_be_bytes());
let tag_at = 128 + 4 + 12;
let size = 16 + 12 * records + length;
b.extend(b"desc");
b.extend((tag_at as u32).to_be_bytes());
b.extend(size.to_be_bytes());
b.extend(b"mluc");
b.extend([0; 4]);
b.extend(records.to_be_bytes());
b.extend(12u32.to_be_bytes());
for _ in 0..records {
b.extend(b"enUS");
b.extend(length.to_be_bytes());
b.extend((16 + 12 * records).to_be_bytes());
}
b.extend(vec![0u8; length as usize]);
b
};
assert!(super::icc::localized_text_fits(&profile(3, 64)));
assert!(!super::icc::localized_text_fits(&profile(4096, 64 << 10)));
}
+11
View File
@@ -515,13 +515,24 @@ fn read_acb(bytes: &[u8], fallback: &str) -> Result<SwatchLibrary, String> {
let spot = data.0.windows(8).last() != Some(b"spflproc".as_slice());
let mut lib = SwatchLibrary { name: if title.is_empty() { fallback.into() } else { title }, ..Default::default() };
let mut names = UniqueNames::default();
// Every name gets the prefix and the suffix, and is kept twice (the swatch's and the set of
// names taken): a megabyte book of long affixes and many colors came to hundreds of MB.
let mut kept = 0usize;
for (name, color) in colors.into_iter().filter(|(n, _)| !n.trim().is_empty()) {
kept = kept.saturating_add(2 * (prefix.len() + name.len() + suffix.len()));
if kept > ACB_MAX_NAME_BYTES {
return Err(format!("the color book's names take more than {} MB", ACB_MAX_NAME_BYTES >> 20));
}
let name = names.unique(format!("{prefix}{name}{suffix}").trim().to_string());
lib.swatches.push(Swatch { name, paint: Paint::solid(color), global: spot, spot });
}
Ok(lib)
}
/// Most bytes a color book's full color names (with its prefix and suffix) may take, counted
/// twice as they are kept twice.
const ACB_MAX_NAME_BYTES: usize = 64 << 20;
/// The swatch of an `.ase` color block, `None` for a model other than RGB, CMYK, Lab and Gray.
/// Components are clamped to their ranges.
fn ase_swatch(body: &mut AseData) -> Result<Option<Swatch>, String> {
+46 -13
View File
@@ -7,6 +7,7 @@
//! directions.
use std::io::Cursor;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex, PoisonError};
use vectorcraft_geom::Point;
@@ -26,6 +27,15 @@ const RELIEF: f64 = 1.0;
/// How many decoded textures [`texture`] keeps.
const KEEP: usize = 4;
/// Most bytes of gray levels the loaded textures alive at once may hold, wherever they are kept
/// (the effects of every drawing hold theirs): a texture that would pass it isn't decoded. Each
/// one is decoded within [`vectorcraft_doc::MAX_RASTER_ALLOC`], but a document's many Glass
/// effects each kept their own.
const MAX_LIVE: usize = 512 << 20;
/// Bytes of gray levels the [`LoadedTexture`]s alive hold ([`MAX_LIVE`]).
static LIVE: AtomicUsize = AtomicUsize::new(0);
/// A loaded texture: `width` × `height` gray levels, row by row from the top.
#[derive(Debug, PartialEq, Eq)]
pub struct LoadedTexture {
@@ -34,7 +44,19 @@ pub struct LoadedTexture {
gray: Vec<u8>,
}
impl Drop for LoadedTexture {
fn drop(&mut self) {
LIVE.fetch_sub(self.gray.len(), Ordering::Relaxed);
}
}
impl LoadedTexture {
/// A texture of `gray`, counted in [`LIVE`].
fn new(width: u32, height: u32, gray: Vec<u8>) -> Self {
LIVE.fetch_add(gray.len(), Ordering::Relaxed);
Self { width, height, gray }
}
/// The texture of `width` × `height` straight (not premultiplied) RGBA pixels: each pixel's
/// luma over middle gray, as much as its alpha covers it (a transparent pixel is middle gray).
/// `None` when the image is empty or `rgba` doesn't hold exactly that many pixels.
@@ -52,7 +74,7 @@ impl LoadedTexture {
((a * l + (1.0 - a) * 0.5) * 255.0).round().clamp(0.0, 255.0) as u8
})
.collect();
Some(Self { width, height, gray })
Some(Self::new(width, height, gray))
}
/// The size in pixels.
@@ -71,14 +93,23 @@ impl LoadedTexture {
/// The texture of a PNG's gray levels, decoded within [`vectorcraft_doc::MAX_RASTER_ALLOC`];
/// `None` when `png` isn't a PNG or holds no pixels.
/// `None` too when its gray levels would take the live textures past [`MAX_LIVE`].
fn from_png(png: &[u8]) -> Option<Self> {
let mut reader = image::ImageReader::with_format(Cursor::new(png), image::ImageFormat::Png);
let mut limits = image::Limits::default();
limits.max_alloc = Some(vectorcraft_doc::MAX_RASTER_ALLOC);
reader.limits(limits);
let img = reader.decode().ok()?.to_luma8();
let reader = || {
let mut reader = image::ImageReader::with_format(Cursor::new(png), image::ImageFormat::Png);
let mut limits = image::Limits::default();
limits.max_alloc = Some(vectorcraft_doc::MAX_RASTER_ALLOC);
reader.limits(limits);
reader
};
let (w, h) = reader().into_dimensions().ok()?;
let gray = (w as usize).checked_mul(h as usize)?;
if LIVE.load(Ordering::Relaxed).saturating_add(gray) > MAX_LIVE {
return None;
}
let img = reader().decode().ok()?.to_luma8();
let (width, height) = img.dimensions();
(width > 0 && height > 0).then(|| Self { width, height, gray: img.into_raw() })
(width > 0 && height > 0).then(|| Self::new(width, height, img.into_raw()))
}
/// The surface's height (points) at `p` (points at 100 %, from the object's center): the gray
@@ -99,24 +130,26 @@ impl LoadedTexture {
}
}
/// The textures [`texture`] decoded last, with their stored form, the newest last.
static DECODED: Mutex<Vec<(String, Arc<LoadedTexture>)>> = Mutex::new(Vec::new());
/// The textures [`texture`] decoded last, with their PNG, the newest last.
static DECODED: Mutex<Vec<(Vec<u8>, Arc<LoadedTexture>)>> = Mutex::new(Vec::new());
/// The texture a [`TEXTURE_IMAGE`] value holds; `None` when it isn't base64 of a PNG. The last
/// [`KEEP`] textures decoded are kept for the next call.
/// [`KEEP`] textures decoded are kept for the next call, found by their PNG: values that differ
/// only in how the base64 is spaced share one.
pub fn texture(stored: &str) -> Option<Arc<LoadedTexture>> {
let png = base64_decode(stored)?;
let mut decoded = DECODED.lock().unwrap_or_else(PoisonError::into_inner);
if let Some(i) = decoded.iter().position(|(s, _)| s == stored) {
if let Some(i) = decoded.iter().position(|(p, _)| *p == png) {
let entry = decoded.remove(i);
let t = entry.1.clone();
decoded.push(entry);
return Some(t);
}
let t = Arc::new(LoadedTexture::from_png(&base64_decode(stored)?)?);
let t = Arc::new(LoadedTexture::from_png(&png)?);
if decoded.len() >= KEEP {
decoded.remove(0);
}
decoded.push((stored.to_string(), t.clone()));
decoded.push((png, t.clone()));
Some(t)
}
+22
View File
@@ -12,6 +12,24 @@ use crate::{GeomContext, apply_geometry_with, is_geometry, merged_params};
pub const REVOLVE: &str = "threeD.revolve";
/// Most bytes the copies of the object's paint in the faces may take together. Each face and the
/// seam stroke around it gets a copy of the paint, so a gradient of many stops times tens of
/// thousands of faces took gigabytes; past this the source path is drawn.
const MAX_PAINT_COPIES: usize = 256 << 20;
/// About the bytes one copy of `paint` takes.
fn paint_bytes(paint: &Paint) -> usize {
use std::mem::size_of;
use vectorcraft_doc::color::{FreeformPoint, GradientStop};
let own = size_of::<Paint>();
let Paint::Gradient(g) = paint else { return own };
let freeform = g
.freeform
.as_ref()
.map_or(0, |f| f.points.len() * size_of::<FreeformPoint>() + f.lines.iter().map(|l| l.len() * size_of::<usize>()).sum::<usize>());
own + size_of::<vectorcraft_doc::color::GradientPaint>() + g.gradient.stops.len() * size_of::<GradientStop>() + freeform
}
fn visible(fx: &[Effect]) -> bool {
fx.iter().any(|e| e.visible && e.id == REVOLVE)
}
@@ -93,6 +111,10 @@ fn surface(
let options = revolve_options(&merged_params(REVOLVE, &fx.params));
let colored = options.light_color != [1.0; 3] || options.lights.is_some();
let faces = vectorcraft_three_d::revolve(path, b, options).ok()?;
// A face's fill, its shadow and their seam strokes: up to four copies of the paint.
if faces.len().saturating_mul(4).saturating_mul(paint_bytes(&paint)) > MAX_PAINT_COPIES {
return None;
}
// Visibility is an expansion option, never a change to live geometry or export baking.
// Keep all faces for paints whose coverage we cannot prove opaque. A bounded visibility
// failure also retains the complete surface rather than losing any of the user's art.
+3 -1
View File
@@ -58,7 +58,9 @@ pub(super) fn flatten(segs: &[PathSeg], closed: bool, tol: f64) -> (Vec<Point>,
};
match seg {
PathSeg::Line(l) => push(l.p1),
_ => kurbo::flatten([PathEl::MoveTo(seg.start()), seg.as_path_el()], tol, |el| {
// Bounded per curve: a projection can blow a small curve up enough that kurbo sizes
// an enormous buffer from it.
_ => vectorcraft_geom::flatten([PathEl::MoveTo(seg.start()), seg.as_path_el()], tol, |el| {
if let PathEl::LineTo(p) = el {
push(p);
}
+69
View File
@@ -26,6 +26,53 @@ pub use projective::Homography;
/// Tolerance used when comparing handle positions to anchor positions.
pub const EPS: f64 = 1e-9;
/// Most line segments [`flatten`] makes of one curve, whatever its size and the tolerance.
pub const MAX_FLATTEN_PER_CURVE: f64 = 4096.0;
/// [`kurbo::flatten`], each curve's tolerance raised so that it makes at most about
/// [`MAX_FLATTEN_PER_CURVE`] segments. kurbo sizes its work and a buffer from the curve's
/// coordinates before producing anything, so a small curve with far-out handles (from a file, or
/// scaled up by a view or a projection) asked for gigabytes. The segments a curve needs grow with
/// the square root of its control polygon's length over the tolerance; that ratio is held under
/// the square of the cap. A curve whose points aren't finite is drawn as a line to its end.
pub fn flatten(path: impl IntoIterator<Item = PathEl>, tolerance: f64, mut callback: impl FnMut(PathEl)) {
let mut last = None;
let mut start = None;
for el in path {
let (from, to, length) = match el {
PathEl::MoveTo(p) => {
(last, start) = (Some(p), Some(p));
callback(el);
continue;
}
PathEl::LineTo(p) => {
last = Some(p);
callback(el);
continue;
}
PathEl::ClosePath => {
last = start;
callback(el);
continue;
}
PathEl::QuadTo(a, b) => (last, b, last.map(|p| p.distance(a) + a.distance(b))),
PathEl::CurveTo(a, b, c) => (last, c, last.map(|p| p.distance(a) + a.distance(b) + b.distance(c))),
};
last = Some(to);
let (Some(from), Some(length)) = (from, length) else { continue };
if !length.is_finite() {
callback(PathEl::LineTo(to));
continue;
}
let tol = tolerance.max(length / (MAX_FLATTEN_PER_CURVE * MAX_FLATTEN_PER_CURVE));
kurbo::flatten([PathEl::MoveTo(from), el], tol, |e| {
if !matches!(e, PathEl::MoveTo(_)) {
callback(e);
}
});
}
}
/// Union of two optional rects.
pub fn union_opt(a: Option<Rect>, b: Option<Rect>) -> Option<Rect> {
match (a, b) {
@@ -131,6 +178,28 @@ pub fn reference_point(r: Rect, index: usize) -> Point {
mod tests {
use super::*;
/// A tiny curve with far-out handles flattens into a bounded number of segments; an ordinary
/// one exactly as kurbo flattens it.
#[test]
fn flatten_bounds_the_segments_of_one_curve() {
let count = |path: &[PathEl], tol: f64| {
let mut n = 0usize;
flatten(path.iter().copied(), tol, |_| n += 1);
n
};
let wild = [PathEl::MoveTo(Point::ZERO), PathEl::CurveTo(Point::new(1e15, -1e15), Point::new(-1e15, 1e15), Point::new(1.0, 0.0))];
assert!(count(&wild, 0.25) < 3 * MAX_FLATTEN_PER_CURVE as usize);
let plain =
[PathEl::MoveTo(Point::ZERO), PathEl::CurveTo(Point::new(0.0, 50.0), Point::new(100.0, 50.0), Point::new(100.0, 0.0)), PathEl::ClosePath];
let mut ours = vec![];
flatten(plain.iter().copied(), 0.25, |e| ours.push(e));
let mut theirs = vec![];
kurbo::flatten(plain.iter().copied(), 0.25, |e| theirs.push(e));
assert_eq!(ours, theirs);
let nan = [PathEl::MoveTo(Point::ZERO), PathEl::QuadTo(Point::new(f64::NAN, 0.0), Point::new(1.0, 1.0))];
assert_eq!(count(&nan, 0.25), 2);
}
#[test]
fn constrain_axes_are_exact_and_whole_turns_are_none() {
for d in [0.0, 360.0, -360.0, 720.0, f64::NAN, f64::INFINITY] {
+27 -7
View File
@@ -29,6 +29,10 @@ pub const WALL_TIME_MS: u64 = 5_000;
/// Cached results kept before the cache starts over.
const CACHE_SIZE: usize = 512;
/// Anchors the cached inputs and results may hold together before the cache starts over (about
/// 50 MB). A plug-in can return far more than it was given, so a count of results alone let a few
/// hundred small inputs keep gigabytes. A result past a quarter of this isn't kept.
const CACHE_ANCHORS: usize = 1 << 20;
/// The plug-in id of effect id `id` (`plugin.<id>`).
pub fn plugin_id(effect_id: &str) -> Option<&str> {
@@ -107,9 +111,18 @@ impl Key {
}
}
type Cache = HashMap<u64, (Key, Option<PathData>)>;
#[derive(Default)]
struct Cache {
entries: HashMap<u64, (Key, Option<PathData>)>,
/// The anchors of the entries' inputs and results ([`CACHE_ANCHORS`]).
anchors: usize,
}
static CACHE: Mutex<Option<Cache>> = Mutex::new(None);
fn anchors(p: &PathData) -> usize {
p.subpaths.iter().map(|sp| sp.anchors.len()).sum()
}
static ERRORS: Mutex<Option<HashMap<String, String>>> = Mutex::new(None);
/// The last error of live-effect plug-in `id` (cleared when it next succeeds).
@@ -142,7 +155,7 @@ pub fn apply(id: &str, params: &Value, path: &PathData, bounds: Rect, rule: Fill
path: path.clone(),
};
let h = key.hash64();
if let Some((k, out)) = CACHE.lock().unwrap_or_else(|e| e.into_inner()).as_ref().and_then(|c| c.get(&h))
if let Some((k, out)) = CACHE.lock().unwrap_or_else(|e| e.into_inner()).as_ref().and_then(|c| c.entries.get(&h))
&& *k == key
{
return out.clone();
@@ -155,12 +168,19 @@ pub fn apply(id: &str, params: &Value, path: &PathData, bounds: Rect, rule: Fill
}
note(id, &r);
let out = r.ok();
let mut cache = CACHE.lock().unwrap_or_else(|e| e.into_inner());
let cache = cache.get_or_insert_with(HashMap::new);
if cache.len() >= CACHE_SIZE {
cache.clear();
let size = anchors(&key.path) + out.as_ref().map_or(0, anchors);
if size > CACHE_ANCHORS / 4 {
return out;
}
cache.insert(h, (key, out.clone()));
let mut cache = CACHE.lock().unwrap_or_else(|e| e.into_inner());
let cache = cache.get_or_insert_with(Cache::default);
if cache.entries.len() >= CACHE_SIZE || cache.anchors + size > CACHE_ANCHORS {
*cache = Cache::default();
}
if let Some((old, old_out)) = cache.entries.insert(h, (key, out.clone())) {
cache.anchors = cache.anchors.saturating_sub(anchors(&old.path) + old_out.as_ref().map_or(0, anchors));
}
cache.anchors += size;
out
}
+14 -6
View File
@@ -46,6 +46,11 @@ pub struct Limits {
/// Wall-clock budget for one run, in milliseconds. Native builds only: the web build has no
/// monotonic clock in `std`, so the fuel budget alone bounds it there.
pub wall_time_ms: u64,
/// The most a heavy filter ([`crate::Budget::Heavy`]) is given instead of `fuel_base` and
/// `wall_time_ms`: what the host allows, which the module's manifest can ask for but never
/// raise. Set them to `fuel_base` and `wall_time_ms` to give heavy filters nothing more.
pub heavy_fuel: u64,
pub heavy_wall_time_ms: u64,
}
impl Default for Limits {
@@ -58,6 +63,8 @@ impl Default for Limits {
fuel_per_byte: 4_000,
max_recursion_depth: 1024,
wall_time_ms: 60_000,
heavy_fuel: HEAVY_FUEL,
heavy_wall_time_ms: HEAVY_WALL_TIME_MS,
}
}
}
@@ -170,11 +177,11 @@ impl Plugin {
e => e,
})?;
p.manifest = Manifest::parse(&text)?;
// A heavy filter's runs get the larger budget, which the host lets the user allow or
// refuse before installing it.
// A heavy filter's runs get the larger budget the host allows for one, which the host
// lets the user allow or refuse before installing it.
if p.manifest.budget == crate::Budget::Heavy {
p.limits.fuel_base = p.limits.fuel_base.max(HEAVY_FUEL);
p.limits.wall_time_ms = p.limits.wall_time_ms.max(HEAVY_WALL_TIME_MS);
p.limits.fuel_base = p.limits.fuel_base.max(p.limits.heavy_fuel);
p.limits.wall_time_ms = p.limits.wall_time_ms.max(p.limits.heavy_wall_time_ms);
}
// The other exports must exist with the right types.
live.func::<i32, i32>("vc_alloc")?;
@@ -217,8 +224,9 @@ impl Plugin {
StoreLimitsBuilder::new().memory_size(self.limits.max_memory_bytes).memories(1).tables(4).table_elements(100_000).instances(1).build();
let mut store = Store::new(&self.engine, limits);
store.limiter(|l| l);
// The start function (if any) runs on this fuel.
store.set_fuel(self.limits.fuel_base).map_err(wasm_err)?;
// The start function (if any) runs on one slice of fuel: it runs before `call` can check
// the deadline or a cancel between slices, so a heavy budget would let it run for minutes.
store.set_fuel(self.limits.fuel_base.min(FUEL_SLICE)).map_err(wasm_err)?;
let linker = Linker::<StoreLimits>::new(&self.engine);
let instance = linker.instantiate_and_start(&mut store, &self.module).map_err(wasm_err)?;
let memory =
+3 -1
View File
@@ -3442,7 +3442,9 @@ fn recent_row(ui: &mut Ui, path: &str) -> egui::Response {
}
fn kurbo_flatten(p: &BezPath, tol: f64, f: &mut impl FnMut(PathEl)) {
vectorcraft_geom::kurbo::flatten(p.elements().iter().copied(), tol, f);
// Bounded per curve: a document's far-out handles, zoomed onto the screen, made kurbo
// reserve gigabytes for the selection outline.
vectorcraft_geom::flatten(p.elements().iter().copied(), tol, f);
}
/// The rendered raster `img` as an egui image. Both are premultiplied RGBA8, so its bytes are copied