This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new 209884967 fix(rust): fix several panics detected by cargo-fuzz (#3483)
209884967 is described below
commit 209884967501428858dc1f73d2c5fc75fe8f3a7f
Author: Peiyang He <[email protected]>
AuthorDate: Mon Mar 23 06:59:04 2026 -0400
fix(rust): fix several panics detected by cargo-fuzz (#3483)
## Why?
Fix several new panics when feeding corner-case input found by
cargo-fuzz
## What does this PR do?
- In `rust/README.md`, the right command to run all tests seems to be
`cargo test --workspace`. Run `cargo test --features tests` will get:
<img width= "649" height="85" alt="Screenshot 2026-03-15 at 6 16 51 AM"
src="https://github.com/user-attachments/assets/98f52bb3-0227-41f0-8b09-78439cb6531f";
/>
- In `rust/fory-core/src/meta/type_meta.rs`,
-
https://github.com/apache/fory/blob/5fc06f1db45337346db4ed380906c013f1e2f3f7/rust/fory-core/src/meta/type_meta.rs#L645
will panic if `encoding_idx` exceeds the size of `encodings`.
-
https://github.com/apache/fory/blob/5fc06f1db45337346db4ed380906c013f1e2f3f7/rust/fory-core/src/meta/type_meta.rs#L836
will cause OOM if `num_fields` is too large. I limit the max value of
`num_fields` to `i16::MAX` since `field_id` is `i16`
- In `rust/fory-core/src/row/bit_util.rs`, use saturating_add/mul to
prevent potential overflow panic. But would it be better to return error
instead of saturating_add/mul ?🤔
- In `rust/fory-core/src/row/reader.rs`, direct access into slice using
`[]` may cause out-of-bounds panic.
- In `rust/fory-core/src/serializer/collection.rs`,
`rust/fory-core/src/serializer/map.rs` and
`rust/fory-core/src/serializer/primitive_list.rs`, we should check the
remaining bytes in the buffer **before** allocating `Vec`. This can also
prevent OOM.
- In `rust/fory-core/src/serializer/skip.rs`,
`generics.first().unwrap()` and `generics.get(1).unwrap()` will panic if
the size of `generics` is not long enough.
## Related issues
N/A
## AI Contribution Checklist
N/A
## Does this PR introduce any user-facing change?
N/A
## Benchmark
This PR only adds additional check in case of corner-case input and thus
won't has major influence on the performance.
---
rust/README.md | 2 +-
rust/fory-core/src/meta/type_meta.rs | 11 +++++++++++
rust/fory-core/src/serializer/collection.rs | 26 ++++++++++++++++++++++---
rust/fory-core/src/serializer/map.rs | 20 +++++++++++++++----
rust/fory-core/src/serializer/primitive_list.rs | 9 +++++++++
rust/fory-core/src/serializer/skip.rs | 6 ++++++
6 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/rust/README.md b/rust/README.md
index 7f7fac350..aed83968f 100644
--- a/rust/README.md
+++ b/rust/README.md
@@ -670,7 +670,7 @@ cargo build
```bash
# Run all tests
-cargo test --features tests
+cargo test --workspace
# Run specific test
cargo test -p tests --test test_complex_struct
diff --git a/rust/fory-core/src/meta/type_meta.rs
b/rust/fory-core/src/meta/type_meta.rs
index d4ab3a956..1bb5fd135 100644
--- a/rust/fory-core/src/meta/type_meta.rs
+++ b/rust/fory-core/src/meta/type_meta.rs
@@ -60,6 +60,7 @@ use std::collections::HashMap;
use std::rc::Rc;
const SMALL_NUM_FIELDS_THRESHOLD: usize = 0b11111;
+const MAX_TYPE_META_FIELDS: usize = i16::MAX as usize;
const REGISTER_BY_NAME_FLAG: u8 = 0b100000;
const FIELD_NAME_SIZE_THRESHOLD: usize = 0b1111;
/// Marker value in encoding bits to indicate field ID mode (instead of field
name)
@@ -642,6 +643,9 @@ impl TypeMeta {
length as usize
};
let bytes = reader.read_bytes(length)?;
+ if encoding_idx as usize >= encodings.len() {
+ return Err(Error::invalid_data("encoding_index out of bounds"));
+ }
let encoding = encodings[encoding_idx as usize];
decoder.decode(bytes, encoding)
}
@@ -817,6 +821,13 @@ impl TypeMeta {
if num_fields == SMALL_NUM_FIELDS_THRESHOLD {
num_fields += reader.read_varuint32()? as usize;
}
+ // limit the number of fields to prevent potential OOM when creating
Vec<FieldInfo>
+ if num_fields > MAX_TYPE_META_FIELDS {
+ return Err(Error::invalid_data(format!(
+ "too many fields in type meta: {}, max: {}",
+ num_fields, MAX_TYPE_META_FIELDS
+ )));
+ }
let mut type_id;
let mut user_type_id = NO_USER_TYPE_ID;
let namespace;
diff --git a/rust/fory-core/src/serializer/collection.rs
b/rust/fory-core/src/serializer/collection.rs
index 68a6dc6a4..ae16620c7 100644
--- a/rust/fory-core/src/serializer/collection.rs
+++ b/rust/fory-core/src/serializer/collection.rs
@@ -32,6 +32,19 @@ pub const DECL_ELEMENT_TYPE: u8 = 0b100;
// Whether collection elements type same.
pub const IS_SAME_TYPE: u8 = 0b1000;
+fn check_collection_len<T: Serializer>(context: &ReadContext, len: u32) ->
Result<(), Error> {
+ if std::mem::size_of::<T>() == 0 {
+ return Ok(());
+ }
+ let len = len as usize;
+ let remaining = context.reader.slice_after_cursor().len();
+ if len > remaining {
+ let cursor = context.reader.get_cursor();
+ return Err(Error::buffer_out_of_bound(cursor, len, cursor +
remaining));
+ }
+ Ok(())
+}
+
pub fn write_collection_type_info(
context: &mut WriteContext,
collection_type_id: u32,
@@ -245,6 +258,7 @@ where
(header & IS_SAME_TYPE) != 0,
Error::type_error("Type inconsistent, target type is not polymorphic")
);
+ check_collection_len::<T>(context, len)?;
if !has_null {
(0..len)
.map(|_| T::fory_read_data(context))
@@ -284,6 +298,7 @@ where
(header & IS_SAME_TYPE) != 0,
Error::type_error("Type inconsistent, target type is not polymorphic")
);
+ check_collection_len::<T>(context, len)?;
let mut vec = Vec::with_capacity(len as usize);
if !has_null {
for _ in 0..len {
@@ -320,14 +335,14 @@ where
} else {
RefMode::None
};
-
- let mut vec = Vec::with_capacity(len as usize);
if is_same_type {
let type_info = if !is_declared {
context.read_any_type_info()?
} else {
T::fory_get_type_info(context.get_type_resolver())?
};
+ check_collection_len::<T>(context, len)?;
+ let mut vec = Vec::with_capacity(len as usize);
if elem_ref_mode == RefMode::None {
for _ in 0..len {
vec.push(T::fory_read_with_type_info(
@@ -345,12 +360,15 @@ where
)?);
}
}
+ Ok(vec)
} else {
+ check_collection_len::<T>(context, len)?;
+ let mut vec = Vec::with_capacity(len as usize);
for _ in 0..len {
vec.push(T::fory_read(context, elem_ref_mode, true)?);
}
+ Ok(vec)
}
- Ok(vec)
}
/// Slow but versatile collection deserialization for dynamic trait object and
shared/circular reference.
@@ -382,6 +400,7 @@ where
} else {
T::fory_get_type_info(context.get_type_resolver())?
};
+ check_collection_len::<T>(context, len)?;
// All elements are same type
if elem_ref_mode == RefMode::None {
// No null elements, no tracking
@@ -395,6 +414,7 @@ where
.collect::<Result<C, Error>>()
}
} else {
+ check_collection_len::<T>(context, len)?;
(0..len)
.map(|_| T::fory_read(context, elem_ref_mode, true))
.collect::<Result<C, Error>>()
diff --git a/rust/fory-core/src/serializer/map.rs
b/rust/fory-core/src/serializer/map.rs
index 4e90a1c3d..f8c7997c0 100644
--- a/rust/fory-core/src/serializer/map.rs
+++ b/rust/fory-core/src/serializer/map.rs
@@ -34,6 +34,16 @@ const TRACKING_VALUE_REF: u8 = 0b1000;
pub const VALUE_NULL: u8 = 0b10000;
pub const DECL_VALUE_TYPE: u8 = 0b100000;
+fn check_map_len(context: &ReadContext, len: u32) -> Result<(), Error> {
+ let len = len as usize;
+ let remaining = context.reader.slice_after_cursor().len();
+ if len > remaining {
+ let cursor = context.reader.get_cursor();
+ return Err(Error::buffer_out_of_bound(cursor, len, cursor +
remaining));
+ }
+ Ok(())
+}
+
fn write_chunk_size(context: &mut WriteContext, header_offset: usize, size:
u8) {
context.writer.set_bytes(header_offset + 1, &[size]);
}
@@ -547,10 +557,10 @@ impl<K: Serializer + ForyDefault + Eq + std::hash::Hash,
V: Serializer + ForyDef
fn fory_read_data(context: &mut ReadContext) -> Result<Self, Error> {
let len = context.reader.read_varuint32()?;
- let mut map = HashMap::<K, V>::with_capacity(len as usize);
if len == 0 {
- return Ok(map);
+ return Ok(HashMap::new());
}
+ check_map_len(context, len)?;
if K::fory_is_polymorphic()
|| K::fory_is_shared_ref()
|| V::fory_is_polymorphic()
@@ -559,6 +569,7 @@ impl<K: Serializer + ForyDefault + Eq + std::hash::Hash, V:
Serializer + ForyDef
let map: HashMap<K, V> = HashMap::with_capacity(len as usize);
return read_hashmap_data_dyn_ref(context, map, len);
}
+ let mut map = HashMap::<K, V>::with_capacity(len as usize);
let mut len_counter = 0;
loop {
if len_counter == len {
@@ -698,10 +709,11 @@ impl<K: Serializer + ForyDefault + Ord + std::hash::Hash,
V: Serializer + ForyDe
fn fory_read_data(context: &mut ReadContext) -> Result<Self, Error> {
let len = context.reader.read_varuint32()?;
- let mut map = BTreeMap::<K, V>::new();
if len == 0 {
- return Ok(map);
+ return Ok(BTreeMap::new());
}
+ check_map_len(context, len)?;
+ let mut map = BTreeMap::<K, V>::new();
if K::fory_is_polymorphic()
|| K::fory_is_shared_ref()
|| V::fory_is_polymorphic()
diff --git a/rust/fory-core/src/serializer/primitive_list.rs
b/rust/fory-core/src/serializer/primitive_list.rs
index 2fc8029e3..cd381a7db 100644
--- a/rust/fory-core/src/serializer/primitive_list.rs
+++ b/rust/fory-core/src/serializer/primitive_list.rs
@@ -83,6 +83,15 @@ pub fn fory_read_data<T: Serializer>(context: &mut
ReadContext) -> Result<Vec<T>
if size_bytes % std::mem::size_of::<T>() != 0 {
return Err(Error::invalid_data("Invalid data length"));
}
+ let remaining = context.reader.slice_after_cursor().len();
+ if size_bytes > remaining {
+ let cursor = context.reader.get_cursor();
+ return Err(Error::buffer_out_of_bound(
+ cursor,
+ size_bytes,
+ cursor + remaining,
+ ));
+ }
let len = size_bytes / std::mem::size_of::<T>();
let mut vec: Vec<T> = Vec::with_capacity(len);
diff --git a/rust/fory-core/src/serializer/skip.rs
b/rust/fory-core/src/serializer/skip.rs
index 697158a35..84fbf7647 100644
--- a/rust/fory-core/src/serializer/skip.rs
+++ b/rust/fory-core/src/serializer/skip.rs
@@ -181,6 +181,9 @@ fn skip_collection(context: &mut ReadContext, field_type:
&FieldType) -> Result<
let is_same_type = (header & IS_SAME_TYPE) != 0;
let skip_ref_flag = is_same_type && !has_null;
let is_declared = (header & DECL_ELEMENT_TYPE) != 0;
+ if field_type.generics.is_empty() {
+ return Err(Error::invalid_data("empty generics"));
+ }
let default_elem_type = field_type.generics.first().unwrap();
let (type_info, elem_field_type);
let elem_type = if is_same_type && !is_declared {
@@ -212,6 +215,9 @@ fn skip_map(context: &mut ReadContext, field_type:
&FieldType) -> Result<(), Err
return Ok(());
}
let mut len_counter = 0;
+ if field_type.generics.len() < 2 {
+ return Err(Error::invalid_data("map must have at least 2 generics"));
+ }
let default_key_type = field_type.generics.first().unwrap();
let default_value_type = field_type.generics.get(1).unwrap();
loop {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
