mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-28 03:37:00 -06:00
Security: linear work for shortcut review, gradients, recovery names, include cycles, batch references, DOCX style ids, CAD layouts and long strokes
CodeRabbit findings #2, #16, #31, #103, #114, #124, #162, #179, #182, #183, #185: - Shortcut import review: example names for each code in one pass over the rows. - Gradients: a midpoint stop is mixed from its two stops; freeform recolor samples its increasing positions with one advancing search (`Gradient::sample_increasing_with`). - Recovery: image entry names are looked up by address, and dead ones swept when the map doubles, not on every image. - Script checker: include cycles from strongly connected components (iterative Tarjan), not a reachability row per file. - Batch step references copy at most 256 MB of values for one step. - DOCX: style ids over 256 bytes are ignored where defined and where used. - DXF: layouts keep at most 4 MB of names; a hatch's loops count against the node limit. - Freehand and Liquify: past 1000 samples a drag previews every n/1000 samples, strokes keep at most 100,000 samples, and the stroke kept is previewed in full before it is committed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
bflatastic
co-authored by
Claude Opus 5.5
parent
bc56851904
commit
20bba9ebae
@@ -77,6 +77,10 @@ const MAX_BASED_ON: usize = 32;
|
||||
const MAX_FONTS: usize = 4096;
|
||||
/// The most bytes of a font name kept.
|
||||
const MAX_NAME: usize = 256;
|
||||
/// The most bytes of a style id: a longer one (Word's are under 256 characters) is ignored, both
|
||||
/// where it is defined and where it is used, since every paragraph and run looks its style up by
|
||||
/// it.
|
||||
const MAX_STYLE_ID: usize = 256;
|
||||
/// The most paragraphs placed type takes from one Word document.
|
||||
const MAX_PARAGRAPHS: usize = 100_000;
|
||||
/// The most runs of one character formatting placed type takes from one Word document.
|
||||
@@ -484,7 +488,7 @@ impl Reader {
|
||||
}
|
||||
}
|
||||
Some("style") if self.styles.len() < MAX_STYLES => {
|
||||
let Some(id) = attr(n, "styleId") else { continue };
|
||||
let Some(id) = attr(n, "styleId").filter(|id| id.len() <= MAX_STYLE_ID) else { continue };
|
||||
let default = match attr(n, "type") {
|
||||
Some("paragraph") => Some(&mut self.default_style),
|
||||
Some("character") => Some(&mut self.default_char_style),
|
||||
@@ -495,7 +499,7 @@ impl Reader {
|
||||
}
|
||||
let run = child(n, "rPr").map(|r| self.run_props(r)).unwrap_or_default();
|
||||
let para = child(n, "pPr").map(para_props).unwrap_or_default();
|
||||
let based_on = child(n, "basedOn").and_then(|b| attr(b, "val")).map(str::to_string);
|
||||
let based_on = child(n, "basedOn").and_then(|b| attr(b, "val")).filter(|v| v.len() <= MAX_STYLE_ID).map(str::to_string);
|
||||
self.styles.entry(id.to_string()).or_insert(Style { based_on, run, para });
|
||||
}
|
||||
_ => {}
|
||||
@@ -637,7 +641,12 @@ impl Reader {
|
||||
fn paragraph(&mut self, p: Node, depth: usize) {
|
||||
self.flush();
|
||||
let ppr = child(p, "pPr");
|
||||
let style = ppr.and_then(|n| child(n, "pStyle")).and_then(|n| attr(n, "val")).map(str::to_string).or_else(|| self.default_style.clone());
|
||||
let style = ppr
|
||||
.and_then(|n| child(n, "pStyle"))
|
||||
.and_then(|n| attr(n, "val"))
|
||||
.filter(|v| v.len() <= MAX_STYLE_ID)
|
||||
.map(str::to_string)
|
||||
.or_else(|| self.default_style.clone());
|
||||
let mut para = Para::default();
|
||||
para.apply(&self.default_para);
|
||||
let mut base = Chars::default();
|
||||
@@ -688,7 +697,11 @@ impl Reader {
|
||||
fn run(&mut self, r: Node) {
|
||||
let rpr = child(r, "rPr");
|
||||
let mut chars = self.base;
|
||||
let style = rpr.and_then(|n| child(n, "rStyle")).and_then(|n| attr(n, "val")).or(self.default_char_style.as_deref());
|
||||
let style = rpr
|
||||
.and_then(|n| child(n, "rStyle"))
|
||||
.and_then(|n| attr(n, "val"))
|
||||
.filter(|v| v.len() <= MAX_STYLE_ID)
|
||||
.or(self.default_char_style.as_deref());
|
||||
for s in self.chain(style) {
|
||||
chars.apply(&s.run, true);
|
||||
}
|
||||
|
||||
@@ -589,17 +589,35 @@ static WRITES: Mutex<()> = Mutex::new(());
|
||||
type Named = (Weak<Vec<u8>>, String);
|
||||
|
||||
/// The entry names of image bytes already hashed ([`blob_name`]), so an image is hashed once,
|
||||
/// not at every interval. Bytes behind a live `Weak` never change (`Arc::make_mut` copies them).
|
||||
static NAMES: Mutex<Vec<Named>> = Mutex::new(Vec::new());
|
||||
/// not at every interval, keyed by the bytes' address. Bytes behind a live `Weak` never change
|
||||
/// (`Arc::make_mut` copies them).
|
||||
static NAMES: Mutex<Names> = Mutex::new(Names { by_address: None, live_after_sweep: 0 });
|
||||
|
||||
struct Names {
|
||||
by_address: Option<std::collections::HashMap<usize, Named>>,
|
||||
/// How many entries were live at the last sweep of dead ones: the next sweep waits until there
|
||||
/// are twice as many, so sweeping costs a constant per name, not a pass over all of them.
|
||||
live_after_sweep: usize,
|
||||
}
|
||||
|
||||
/// The name of the entry holding `bytes`: from their content (128 bits of hash and the length), so
|
||||
/// identical images share one entry and one written before isn't written again.
|
||||
fn blob_name(bytes: &Arc<Vec<u8>>) -> String {
|
||||
let mut names = NAMES.lock().unwrap_or_else(|e| e.into_inner());
|
||||
names.retain(|(w, _)| w.strong_count() > 0);
|
||||
if let Some((_, name)) = names.iter().find(|(w, _)| std::ptr::eq(w.as_ptr(), Arc::as_ptr(bytes))) {
|
||||
let mut guard = NAMES.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let Names { by_address, live_after_sweep } = &mut *guard;
|
||||
let names = by_address.get_or_insert_with(Default::default);
|
||||
let address = Arc::as_ptr(bytes) as usize;
|
||||
// A dead entry at this address named other bytes that were freed there before.
|
||||
if let Some((w, name)) = names.get(&address)
|
||||
&& w.strong_count() > 0
|
||||
&& std::ptr::eq(w.as_ptr(), Arc::as_ptr(bytes))
|
||||
{
|
||||
return name.clone();
|
||||
}
|
||||
if names.len() >= (*live_after_sweep * 2).max(64) {
|
||||
names.retain(|_, (w, _)| w.strong_count() > 0);
|
||||
*live_after_sweep = names.len();
|
||||
}
|
||||
let hash = |seed: u8| {
|
||||
let mut h = std::collections::hash_map::DefaultHasher::new();
|
||||
seed.hash(&mut h);
|
||||
@@ -607,7 +625,7 @@ fn blob_name(bytes: &Arc<Vec<u8>>) -> String {
|
||||
h.finish()
|
||||
};
|
||||
let name = format!("img-{:016x}{:016x}-{}{BLOB_EXT}", hash(1), hash(2), bytes.len());
|
||||
names.push((Arc::downgrade(bytes), name.clone()));
|
||||
names.insert(address, (Arc::downgrade(bytes), name.clone()));
|
||||
name
|
||||
}
|
||||
|
||||
|
||||
@@ -11,13 +11,20 @@ use serde_json::Value;
|
||||
/// Most nesting [`resolve`] walks into: deeper parameters are left as they are.
|
||||
const MAX_DEPTH: usize = 64;
|
||||
|
||||
/// The most the values references copy in may weigh together ([`weight`]: about their JSON size),
|
||||
/// for one step's parameters. Each reference copies its value, so a short list of references to
|
||||
/// one large result multiplied it.
|
||||
const MAX_COPIED: usize = 256 << 20;
|
||||
|
||||
/// `params` with every step reference replaced from `results` (the results of the steps run so
|
||||
/// far, in order); an error names a reference that resolves to nothing.
|
||||
/// far, in order); an error names a reference that resolves to nothing, or the reference past
|
||||
/// which the copied values weigh more than [`MAX_COPIED`].
|
||||
pub fn resolve(params: &Value, results: &[Value]) -> Result<Value, String> {
|
||||
resolve_at(params, results, 0)
|
||||
let mut allowance = MAX_COPIED;
|
||||
resolve_at(params, results, 0, &mut allowance)
|
||||
}
|
||||
|
||||
fn resolve_at(v: &Value, results: &[Value], depth: usize) -> Result<Value, String> {
|
||||
fn resolve_at(v: &Value, results: &[Value], depth: usize, allowance: &mut usize) -> Result<Value, String> {
|
||||
if depth > MAX_DEPTH {
|
||||
return Ok(v.clone());
|
||||
}
|
||||
@@ -25,16 +32,49 @@ fn resolve_at(v: &Value, results: &[Value], depth: usize) -> Result<Value, Strin
|
||||
Value::String(s) => match s.strip_prefix("$$") {
|
||||
Some(rest) => Value::String(format!("${rest}")),
|
||||
None => match parse(s) {
|
||||
Some((step, path)) => lookup(s, step, &path, results)?,
|
||||
Some((step, path)) => {
|
||||
let found = lookup(s, step, &path, results)?;
|
||||
*allowance = weight(found, *allowance)
|
||||
.and_then(|w| allowance.checked_sub(w))
|
||||
.ok_or_else(|| format!("`{s}`: the step's references copy in more than {} MB of values", MAX_COPIED >> 20))?;
|
||||
found.clone()
|
||||
}
|
||||
None => v.clone(),
|
||||
},
|
||||
},
|
||||
Value::Array(a) => Value::Array(a.iter().map(|x| resolve_at(x, results, depth + 1)).collect::<Result<_, _>>()?),
|
||||
Value::Object(o) => Value::Object(o.iter().map(|(k, x)| Ok((k.clone(), resolve_at(x, results, depth + 1)?))).collect::<Result<_, String>>()?),
|
||||
Value::Array(a) => Value::Array(a.iter().map(|x| resolve_at(x, results, depth + 1, allowance)).collect::<Result<_, _>>()?),
|
||||
Value::Object(o) => {
|
||||
Value::Object(o.iter().map(|(k, x)| Ok((k.clone(), resolve_at(x, results, depth + 1, allowance)?))).collect::<Result<_, String>>()?)
|
||||
}
|
||||
_ => v.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
/// About the bytes `v` takes as JSON (8 per value, plus its strings and keys); `None` once that
|
||||
/// passes `limit`, without looking further.
|
||||
fn weight(v: &Value, limit: usize) -> Option<usize> {
|
||||
let mut total = 0usize;
|
||||
let mut todo = vec![v];
|
||||
while let Some(v) = todo.pop() {
|
||||
total = total.checked_add(8)?;
|
||||
match v {
|
||||
Value::String(s) => total = total.checked_add(s.len())?,
|
||||
Value::Array(a) => todo.extend(a),
|
||||
Value::Object(o) => {
|
||||
for (k, x) in o {
|
||||
total = total.checked_add(k.len())?;
|
||||
todo.push(x);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
if total > limit {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
Some(total)
|
||||
}
|
||||
|
||||
/// Which step a reference names.
|
||||
#[derive(Debug, PartialEq)]
|
||||
enum Step {
|
||||
@@ -78,7 +118,7 @@ fn parse(s: &str) -> Option<(Step, Vec<Seg>)> {
|
||||
Some((step, path))
|
||||
}
|
||||
|
||||
fn lookup(text: &str, step: Step, path: &[Seg], results: &[Value]) -> Result<Value, String> {
|
||||
fn lookup<'a>(text: &str, step: Step, path: &[Seg], results: &'a [Value]) -> Result<&'a Value, String> {
|
||||
let (i, found) = match step {
|
||||
Step::Index(i) => (i, results.get(i)),
|
||||
Step::Last => (results.len().saturating_sub(1), results.last()),
|
||||
@@ -93,7 +133,7 @@ fn lookup(text: &str, step: Step, path: &[Seg], results: &[Value]) -> Result<Val
|
||||
};
|
||||
cur = next.ok_or_else(|| format!("`{text}`: step {i}'s result has no such value (write $$ for a literal $)"))?;
|
||||
}
|
||||
Ok(cur.clone())
|
||||
Ok(cur)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -109,6 +149,18 @@ mod tests {
|
||||
assert_eq!(resolve(&p, &results).unwrap(), json!({"id": 2, "ids": [6, 5], "nested": {"all": {"ids": [5, 6]}}, "n": 3, "flag": true}));
|
||||
}
|
||||
|
||||
/// References may copy one result many times, but not past [`MAX_COPIED`] together.
|
||||
#[test]
|
||||
fn copies_of_a_large_result_are_bounded() {
|
||||
let big = json!({"text": "x".repeat(MAX_COPIED / 16)});
|
||||
let results = [big];
|
||||
let few = json!(["$0", "$0", "$0"]);
|
||||
assert!(resolve(&few, &results).is_ok());
|
||||
let many = Value::Array(vec![json!("$0"); 32]);
|
||||
let e = resolve(&many, &results).unwrap_err();
|
||||
assert!(e.contains("MB of values"), "{e}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_strings_and_escapes_are_left_alone() {
|
||||
let results = [json!({"id": 2})];
|
||||
|
||||
Reference in New Issue
Block a user