fix(doc): count the values buffered before an object's type (#3587)

NodeKind's reader buffers the fields that come before `type` as
serde_json::Values. VectorCraft writes `type` first, but a crafted file
can put a long array there: a group whose children held 50M numbers before
its type (a 96 KB gzip) peaked at 1.6 GB, and the cost grows with the
array up to MAX_JSON, before any object in it is counted.

The buffer is read through a counting visitor: each JSON value counts
against MAX_BUFFERED_VALUES (32M, 4M on wasm32) as it is read, and values
replayed from the buffer aren't counted twice. load_file reads within it;
the same file is now refused at a 1.1 GB peak.

crates/doc/src/node.rs

@@ -28,7 +28,7 @@ pub use attributes::{ForeignAttribute, ImageMap, MAX_SVG_SCRIPTS, ObjectAttribut
 pub use clip::{ClipPaint, Uniter};
 pub use extras::{Extras, ExtrasMut, NodeExtras};
 pub use ids::{LAYER_COLORS, LayerColor, NodeId};
-pub use kind::{ImageObject, NodeKind};
+pub use kind::{ImageObject, MAX_BUFFERED_VALUES, NodeKind, with_buffered_value_budget};
 pub use live_shape::{LiveShape, Scaling};
 pub use opacity::{Knockout, OpacityMask, whiten};
 use wire::NodeWire;

crates/doc/src/node/kind.rs

@@ -129,6 +129,8 @@ pub enum NodeKind {
 /// anywhere among its fields. Files have it first (it is written first): then the kind is read
 /// straight from the file. Otherwise (hand-written files, values whose keys are sorted) the
 /// fields before `type` are buffered, and only those.
+pub use kind_de::{MAX_BUFFERED_VALUES, with_buffered_value_budget};
+
 mod kind_de {
     use std::fmt;
     use std::sync::Arc;
@@ -237,12 +239,150 @@ mod kind_de {
                     let tag: String = map.next_value()?;
                     return KindDef::deserialize(Tagged { tag, map: Buffered { before: before.into_iter(), value: None, map } });
                 }
-                before.push((key, map.next_value::<serde_json::Value>()?));
+                before.push((key, map.next_value::<Counted>()?.0));
             }
             Err(de::Error::missing_field("type"))
         }
     }
 
+    /// Most JSON values (each number, string, array, object…) the fields read before an object's
+    /// `type` may hold together while [`with_buffered_value_budget`] runs: 32M (about 1 GB as
+    /// `serde_json::Value`s, some 32 bytes each), 4M in the web build. VectorCraft writes `type`
+    /// first, so its files buffer nothing; a reordered file buffers what comes before it, and
+    /// a crafted one could fill memory with values two bytes each in the file before any object in
+    /// them is counted (#3587).
+    pub const MAX_BUFFERED_VALUES: usize = if cfg!(target_arch = "wasm32") { 4 << 20 } else { 32 << 20 };
+
+    std::thread_local! {
+        /// Values the fields buffered on this thread may still hold, and the budget they come from.
+        static VALUES_LEFT: std::cell::Cell<Option<(usize, usize)>> = const { std::cell::Cell::new(None) };
+        /// Buffered fields being read back: what they hold was counted when it was buffered.
+        static REPLAYING: std::cell::Cell<u32> = const { std::cell::Cell::new(0) };
+    }
+
+    /// Run `f` (which reads objects with serde) letting the fields it buffers before an object's
+    /// `type` hold at most `max` values together; buffering more fails.
+    pub fn with_buffered_value_budget<T>(max: usize, f: impl FnOnce() -> T) -> T {
+        struct Restore(Option<(usize, usize)>);
+        impl Drop for Restore {
+            fn drop(&mut self) {
+                VALUES_LEFT.set(self.0);
+            }
+        }
+        let _restore = Restore(VALUES_LEFT.replace(Some((max, max))));
+        f()
+    }
+
+    /// While buffered fields are read back ([`REPLAYING`]).
+    struct Replaying;
+
+    impl Replaying {
+        fn enter() -> Self {
+            REPLAYING.set(REPLAYING.get().saturating_add(1));
+            Replaying
+        }
+    }
+
+    impl Drop for Replaying {
+        fn drop(&mut self) {
+            REPLAYING.set(REPLAYING.get().saturating_sub(1));
+        }
+    }
+
+    /// Count one buffered value against [`with_buffered_value_budget`].
+    fn count<E: de::Error>() -> Result<(), E> {
+        if REPLAYING.get() > 0 {
+            return Ok(());
+        }
+        if let Some((left, max)) = VALUES_LEFT.get() {
+            let left = left.checked_sub(1).ok_or_else(|| E::custom(format!("an object has more than {max} values before its type")))?;
+            VALUES_LEFT.set(Some((left, max)));
+        }
+        Ok(())
+    }
+
+    /// A field read before `type`, as a `serde_json::Value` counted value by value as it is read,
+    /// so the budget stops a large one before it is all in memory.
+    struct Counted(serde_json::Value);
+
+    impl<'de> Deserialize<'de> for Counted {
+        fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
+            d.deserialize_any(CountedVisitor)
+        }
+    }
+
+    struct CountedVisitor;
+
+    impl<'de> Visitor<'de> for CountedVisitor {
+        type Value = Counted;
+
+        fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            f.write_str("a JSON value")
+        }
+
+        fn visit_bool<E: de::Error>(self, v: bool) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(serde_json::Value::Bool(v)))
+        }
+
+        fn visit_i64<E: de::Error>(self, v: i64) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(v.into()))
+        }
+
+        fn visit_u64<E: de::Error>(self, v: u64) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(v.into()))
+        }
+
+        fn visit_f64<E: de::Error>(self, v: f64) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(serde_json::Number::from_f64(v).map_or(serde_json::Value::Null, serde_json::Value::Number)))
+        }
+
+        fn visit_str<E: de::Error>(self, v: &str) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(serde_json::Value::String(v.to_owned())))
+        }
+
+        fn visit_string<E: de::Error>(self, v: String) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(serde_json::Value::String(v)))
+        }
+
+        fn visit_none<E: de::Error>(self) -> Result<Counted, E> {
+            self.visit_unit()
+        }
+
+        fn visit_unit<E: de::Error>(self) -> Result<Counted, E> {
+            count()?;
+            Ok(Counted(serde_json::Value::Null))
+        }
+
+        fn visit_some<D: Deserializer<'de>>(self, d: D) -> Result<Counted, D::Error> {
+            Counted::deserialize(d)
+        }
+
+        fn visit_seq<S: de::SeqAccess<'de>>(self, mut seq: S) -> Result<Counted, S::Error> {
+            count()?;
+            let mut out = Vec::new();
+            while let Some(Counted(v)) = seq.next_element()? {
+                out.push(v);
+            }
+            Ok(Counted(serde_json::Value::Array(out)))
+        }
+
+        fn visit_map<M: MapAccess<'de>>(self, mut map: M) -> Result<Counted, M::Error> {
+            count()?;
+            let mut out = serde_json::Map::new();
+            while let Some(key) = map.next_key::<String>()? {
+                let Counted(v) = map.next_value()?;
+                out.insert(key, v);
+            }
+            Ok(Counted(serde_json::Value::Object(out)))
+        }
+    }
+
     /// The fields read before `type` (buffered), then the rest of the object.
     struct Buffered<A> {
         before: std::vec::IntoIter<(String, serde_json::Value)>,
@@ -265,7 +405,11 @@ mod kind_de {
 
         fn next_value_seed<V: DeserializeSeed<'de>>(&mut self, seed: V) -> Result<V::Value, A::Error> {
             match self.value.take() {
-                Some(value) => seed.deserialize(value).map_err(de::Error::custom),
+                Some(value) => {
+                    // Counted when it was read: the objects it holds buffer it again as they read it.
+                    let _replaying = Replaying::enter();
+                    seed.deserialize(value).map_err(de::Error::custom)
+                }
                 None => self.map.next_value_seed(seed),
             }
         }

crates/doc/src/node/tests.rs

@@ -449,3 +449,21 @@ fn objects_are_read_within_the_node_budget() {
     assert!(with_node_budget(1, read).is_err());
     assert!(with_node_budget(3, read).is_ok());
 }
+
+/// #3587: fields read before an object's `type` are buffered value by value within
+/// [`with_buffered_value_budget`], each counted once: a group read back from the buffer buffers
+/// its own fields again without counting them twice. VectorCraft writes `type` first, which
+/// buffers nothing.
+#[test]
+fn fields_before_the_type_are_buffered_within_the_value_budget() {
+    let reordered = r#"{"id":1,"kind":{"children":[{"id":2,"kind":{"children":[],"type":"group"}}],"type":"group"}}"#;
+    let read = |json: &str| serde_json::from_str::<Node>(json);
+    // The children's array, the inner object, its id, its kind, the kind's empty children and
+    // its type: six values.
+    assert!(with_buffered_value_budget(6, || read(reordered)).is_ok());
+    let why = with_buffered_value_budget(5, || read(reordered)).unwrap_err().to_string();
+    assert!(why.contains("more than 5 values before its type"), "{why}");
+    let written = serde_json::to_string(&Node::group(NodeId(1), vec![Arc::new(Node::group(NodeId(2), vec![]))])).unwrap();
+    assert!(with_buffered_value_budget(0, || read(&written)).is_ok(), "a saved object buffers nothing: {written}");
+    assert!(read(reordered).is_ok(), "no budget outside a read");
+}

crates/format/src/lib.rs

@@ -552,16 +552,22 @@ pub fn load_info(bytes: &[u8]) -> Result<(Document, FileInfo), FormatError> {
 
 /// [`load`], with everything else the file carries.
 pub fn load_file(bytes: &[u8]) -> Result<NativeFile, FormatError> {
-    load_file_within(bytes, vectorcraft_doc::node::MAX_READ_NODES)
+    load_file_within(bytes, vectorcraft_doc::node::MAX_READ_NODES, vectorcraft_doc::node::MAX_BUFFERED_VALUES)
 }
 
-/// [`load_file`] reading at most `max_nodes` objects, so a test can exercise the object budget
-/// without a document of [`vectorcraft_doc::node::MAX_READ_NODES`] objects.
-fn load_file_within(bytes: &[u8], max_nodes: usize) -> Result<NativeFile, FormatError> {
+/// [`load_file`] reading at most `max_nodes` objects and buffering at most `max_values` values
+/// before objects' types, so a test can exercise the budgets without documents that large
+/// ([`vectorcraft_doc::node::MAX_READ_NODES`], [`vectorcraft_doc::node::MAX_BUFFERED_VALUES`]).
+fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result<NativeFile, FormatError> {
     let file = unpack::Unpacked::new(bytes)?;
     // The JSON reader stops at 128 levels (a document's objects use three each), the paths read
-    // at `MAX_READ_ANCHORS` anchors together, and the document at `MAX_READ_NODES` objects.
-    let read = || vectorcraft_doc::node::with_node_budget(max_nodes, || serde_json::from_slice::<File>(file.json()));
+    // at `MAX_READ_ANCHORS` anchors together, the document at `MAX_READ_NODES` objects, and the
+    // fields read before an object's type at `MAX_BUFFERED_VALUES` values.
+    let read = || {
+        vectorcraft_doc::node::with_node_budget(max_nodes, || {
+            vectorcraft_doc::node::with_buffered_value_budget(max_values, || serde_json::from_slice::<File>(file.json()))
+        })
+    };
     let f: File = vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, read).map_err(|e| {
         let why = if e.to_string().contains("recursion limit") { format!("its objects are nested too deep ({e})") } else { e.to_string() };
         FormatError::NotVectorcraft(why)
@@ -797,12 +803,40 @@ mod tests {
         let n = d.node_count();
         let bytes = save_with(&d, &SaveOptions { compress: true, ..SaveOptions::default() }).unwrap();
         assert!(is_compressed(&bytes));
-        assert_eq!(load_file_within(&bytes, n).unwrap().doc.node_count(), n, "a document of exactly the budget opens");
-        let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bytes, n - 1) else { panic!("one object past the budget must be refused") };
+        assert_eq!(
+            load_file_within(&bytes, n, vectorcraft_doc::node::MAX_BUFFERED_VALUES).unwrap().doc.node_count(),
+            n,
+            "a document of exactly the budget opens"
+        );
+        let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bytes, n - 1, vectorcraft_doc::node::MAX_BUFFERED_VALUES) else {
+            panic!("one object past the budget must be refused")
+        };
         assert!(why.contains(&format!("more than {} objects", n - 1)), "{why}");
         assert_eq!(load(&bytes).unwrap().node_count(), n, "the real budget opens ordinary documents");
     }
 
+    /// #3587: a reordered file (its objects' fields before their `type`, as a JSON tool that
+    /// sorts keys writes them) still opens; one whose fields before a `type` hold more values than
+    /// the budget, such as a long array of numbers, is refused as they are read.
+    #[test]
+    fn values_buffered_before_an_objects_type_are_capped() {
+        let mut d = Document::new(100.0, 100.0);
+        let l = d.add_layer(None);
+        let id = d.alloc_id();
+        d.insert(Some(l), 0, Node::path(id, shapes::ellipse(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default_art())).unwrap();
+        // Through `serde_json::Value`, whose objects sort their keys: `children` before `type`.
+        let sorted = serde_json::to_vec(&serde_json::from_slice::<serde_json::Value>(&save(&d, false)).unwrap()).unwrap();
+        assert_eq!(load(&sorted).unwrap().node_count(), d.node_count());
+        let mut v: serde_json::Value = serde_json::from_slice(&save(&d, false)).unwrap();
+        let zeros = vec![0; 1000];
+        v["document"]["layers"][0]["kind"]["children"] = serde_json::json!([{"id": 90, "kind": {"children": zeros, "type": "group"}}]);
+        let bomb = serde_json::to_vec(&v).unwrap();
+        let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bomb, vectorcraft_doc::node::MAX_READ_NODES, 100) else {
+            panic!("past the budget must be refused")
+        };
+        assert!(why.contains("more than 100 values before its type"), "{why}");
+    }
+
     #[test]
     fn rejects_foreign_and_future() {
         assert!(load(b"{}").is_err());