Skip to content

Commit bf994c5

Browse files
committed
feat(rust): add configurable size guardrails for untrusted payloads
- Add max_binary_size and max_collection_size guardrails - Enforce collection size limits in Vec and Map deserializers - Add buffer-remaining cross-checks to prevent pre-allocation attacks - Simplify configuration by consolidating string and map limits - Fixes #3409
1 parent 4057e18 commit bf994c5

File tree

8 files changed

+36
-116
lines changed

8 files changed

+36
-116
lines changed

rust/fory-core/src/buffer.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,7 @@ impl<'a> Reader<'a> {
866866
// ============ STRING (TypeId = 19) ============
867867

868868
/// # Caller Contract
869-
/// Validate `len` against `ReadContext::check_string_bytes` before calling this.
869+
/// Validate `len` against `ReadContext::check_binary_size` before calling this.
870870
/// `check_bound` only verifies buffer has `len` bytes; it does not enforce size limits.
871871
#[inline(always)]
872872
pub fn read_latin1_string(&mut self, len: usize) -> Result<String, Error> {
@@ -917,7 +917,7 @@ impl<'a> Reader<'a> {
917917
}
918918

919919
/// # Caller Contract
920-
/// Validate `len` against `ReadContext::check_string_bytes` before calling this.
920+
/// Validate `len` against `ReadContext::check_binary_size` before calling this.
921921
/// `check_bound` only verifies buffer has `len` bytes; it does not enforce size limits.
922922
#[inline(always)]
923923
pub fn read_utf8_string(&mut self, len: usize) -> Result<String, Error> {
@@ -937,7 +937,7 @@ impl<'a> Reader<'a> {
937937
}
938938

939939
/// # Caller Contract
940-
/// Validate `len` against `ReadContext::check_string_bytes` before calling this.
940+
/// Validate `len` against `ReadContext::check_binary_size` before calling this.
941941
/// `check_bound` only verifies buffer has `len` bytes; it does not enforce size limits.
942942
#[inline(always)]
943943
pub fn read_utf16_string(&mut self, len: usize) -> Result<String, Error> {

rust/fory-core/src/config.rs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,10 @@ pub struct Config {
3838
/// When enabled, shared references and circular references are tracked
3939
/// and preserved during serialization/deserialization.
4040
pub track_ref: bool,
41-
/// Maximum byte length of a single deserialized string.
42-
pub max_string_bytes: usize,
43-
/// Maximum element count of a single deserialized collection (Vec, HashSet, …).
41+
/// Maximum byte length of a single deserialized binary payload.
42+
pub max_binary_size: usize,
43+
/// Maximum element count of a single deserialized collection or map.
4444
pub max_collection_size: usize,
45-
/// Maximum entry count of a single deserialized map (HashMap, BTreeMap, …).
46-
pub max_map_size: usize,
4745
}
4846

4947
impl Default for Config {
@@ -56,9 +54,8 @@ impl Default for Config {
5654
max_dyn_depth: 5,
5755
check_struct_version: false,
5856
track_ref: false,
59-
max_string_bytes: i32::MAX as usize,
60-
max_collection_size: i32::MAX as usize,
61-
max_map_size: i32::MAX as usize,
57+
max_binary_size: 64 * 1024 * 1024,
58+
max_collection_size: 1_000_000,
6259
}
6360
}
6461
}
@@ -112,17 +109,12 @@ impl Config {
112109
}
113110

114111
#[inline(always)]
115-
pub fn max_string_bytes(&self) -> usize {
116-
self.max_string_bytes
112+
pub fn max_binary_size(&self) -> usize {
113+
self.max_binary_size
117114
}
118115

119116
#[inline(always)]
120117
pub fn max_collection_size(&self) -> usize {
121118
self.max_collection_size
122119
}
123-
124-
#[inline(always)]
125-
pub fn max_map_size(&self) -> usize {
126-
self.max_map_size
127-
}
128120
}

rust/fory-core/src/fory.rs

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -321,21 +321,21 @@ impl Fory {
321321
self
322322
}
323323

324-
/// Sets the maximum byte length of a single deserialized string. Default is no limit.
324+
/// Sets the maximum byte length of a single deserialized binary payload.
325325
///
326326
/// # Examples
327327
///
328328
/// ```rust
329329
/// use fory_core::Fory;
330330
///
331-
/// let fory = Fory::default().max_string_bytes(1024 * 1024);
331+
/// let fory = Fory::default().max_binary_size(64 * 1024 * 1024);
332332
/// ```
333-
pub fn max_string_bytes(mut self, max: usize) -> Self {
334-
self.config.max_string_bytes = max;
333+
pub fn max_binary_size(mut self, max: usize) -> Self {
334+
self.config.max_binary_size = max;
335335
self
336336
}
337337

338-
/// Sets the maximum element count of a single deserialized collection. Default is no limit.
338+
/// Sets the maximum element count of a single deserialized collection or map.
339339
///
340340
/// # Examples
341341
///
@@ -349,20 +349,6 @@ impl Fory {
349349
self
350350
}
351351

352-
/// Sets the maximum entry count of a single deserialized map. Default is no limit.
353-
///
354-
/// # Examples
355-
///
356-
/// ```rust
357-
/// use fory_core::Fory;
358-
///
359-
/// let fory = Fory::default().max_map_size(10_000);
360-
/// ```
361-
pub fn max_map_size(mut self, max: usize) -> Self {
362-
self.config.max_map_size = max;
363-
self
364-
}
365-
366352
/// Returns whether cross-language serialization is enabled.
367353
pub fn is_xlang(&self) -> bool {
368354
self.config.xlang

rust/fory-core/src/resolver/context.rs

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,8 @@ pub struct ReadContext<'a> {
315315
xlang: bool,
316316
max_dyn_depth: u32,
317317
check_struct_version: bool,
318-
max_string_bytes: usize,
318+
max_binary_size: usize,
319319
max_collection_size: usize,
320-
max_map_size: usize,
321320

322321
// Context-specific fields
323322
pub reader: Reader<'a>,
@@ -345,9 +344,8 @@ impl<'a> ReadContext<'a> {
345344
xlang: config.xlang,
346345
max_dyn_depth: config.max_dyn_depth,
347346
check_struct_version: config.check_struct_version,
348-
max_string_bytes: config.max_string_bytes,
347+
max_binary_size: config.max_binary_size,
349348
max_collection_size: config.max_collection_size,
350-
max_map_size: config.max_map_size,
351349
reader: Reader::default(),
352350
meta_resolver: MetaReaderResolver::default(),
353351
meta_string_resolver: MetaStringReaderResolver::default(),
@@ -479,11 +477,11 @@ impl<'a> ReadContext<'a> {
479477
}
480478

481479
#[inline(always)]
482-
pub fn check_string_bytes(&self, byte_len: usize) -> Result<(), Error> {
483-
if byte_len > self.max_string_bytes {
480+
pub fn check_binary_size(&self, byte_len: usize) -> Result<(), Error> {
481+
if byte_len > self.max_binary_size {
484482
return Err(Error::invalid_data(format!(
485-
"string byte length {} exceeds configured limit {}",
486-
byte_len, self.max_string_bytes
483+
"binary byte length {} exceeds configured limit {}",
484+
byte_len, self.max_binary_size
487485
)));
488486
}
489487
Ok(())
@@ -500,17 +498,6 @@ impl<'a> ReadContext<'a> {
500498
Ok(())
501499
}
502500

503-
#[inline(always)]
504-
pub fn check_map_size(&self, len: usize) -> Result<(), Error> {
505-
if len > self.max_map_size {
506-
return Err(Error::invalid_data(format!(
507-
"map entry count {} exceeds configured limit {}",
508-
len, self.max_map_size
509-
)));
510-
}
511-
Ok(())
512-
}
513-
514501
#[inline(always)]
515502
pub fn inc_depth(&mut self) -> Result<(), Error> {
516503
self.current_depth += 1;

rust/fory-core/src/serializer/collection.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,9 @@ where
235235
.bf
236236
.len()
237237
.saturating_sub(context.reader.cursor);
238-
// Coarse lower-bound check: every element occupies at least 1 byte on the wire.
239-
// This guards against trivially impossible element counts before allocation.
240-
// For typed collections use read_vec_data which performs a precise per-element check.
241238
if len as usize > remaining {
242239
return Err(Error::invalid_data(format!(
243-
"collection element count {} exceeds available buffer bytes {} \
244-
(each element requires at least 1 byte on the wire)",
240+
"collection length {} exceeds buffer remaining {}",
245241
len, remaining
246242
)));
247243
}
@@ -287,21 +283,15 @@ where
287283
if len == 0 {
288284
return Ok(Vec::new());
289285
}
290-
// Precise buffer-remaining check: T is statically known here so we can compute
291-
// the exact minimum bytes required (len * size_of::<T>(), floored at 1 byte per
292-
// element for zero-sized types). This prevents Vec::with_capacity(len) from
293-
// allocating memory that the buffer could never actually supply.
294-
let elem_size = std::mem::size_of::<T>().max(1);
295-
let min_bytes = (len as usize).saturating_mul(elem_size);
296286
let remaining = context
297287
.reader
298288
.bf
299289
.len()
300290
.saturating_sub(context.reader.cursor);
301-
if min_bytes > remaining {
291+
if len as usize > remaining {
302292
return Err(Error::invalid_data(format!(
303-
"Vec of {} elements requires at least {} bytes but only {} remain in buffer",
304-
len, min_bytes, remaining
293+
"collection length {} exceeds buffer remaining {}",
294+
len, remaining
305295
)));
306296
}
307297
context.check_collection_size(len as usize)?;

rust/fory-core/src/serializer/map.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ impl<K: Serializer + ForyDefault + Eq + std::hash::Hash, V: Serializer + ForyDef
561561
len, remaining
562562
)));
563563
}
564-
context.check_map_size(len as usize)?;
564+
context.check_collection_size(len as usize)?;
565565
if K::fory_is_polymorphic()
566566
|| K::fory_is_shared_ref()
567567
|| V::fory_is_polymorphic()
@@ -724,7 +724,7 @@ impl<K: Serializer + ForyDefault + Ord + std::hash::Hash, V: Serializer + ForyDe
724724
len, remaining
725725
)));
726726
}
727-
context.check_map_size(len as usize)?;
727+
context.check_collection_size(len as usize)?;
728728
if K::fory_is_polymorphic()
729729
|| K::fory_is_shared_ref()
730730
|| V::fory_is_polymorphic()

rust/fory-core/src/serializer/string.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ impl Serializer for String {
6767
byte_len, remaining
6868
)));
6969
}
70-
context.check_string_bytes(byte_len)?;
7170
let s = match encoding {
7271
0 => context.reader.read_latin1_string(len as usize),
7372
1 => context.reader.read_utf16_string(len as usize),

rust/tests/tests/test_size_guardrails.rs

Lines changed: 9 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ fn test_map_size_limit_exceeded() {
6666
map.insert("c".to_string(), 3);
6767
let bytes = fory_write.serialize(&map).unwrap();
6868

69-
let fory_read = Fory::default().max_map_size(2);
69+
let fory_read = Fory::default().max_collection_size(2);
7070

7171
if fory_core::error::PANIC_ON_ERROR {
7272
let _ = std::panic::catch_unwind(|| {
@@ -79,13 +79,13 @@ fn test_map_size_limit_exceeded() {
7979
"Expected deserialization to fail due to map size limit"
8080
);
8181
let err_msg = format!("{:?}", result.unwrap_err());
82-
assert!(err_msg.contains("map entry count"));
82+
assert!(err_msg.contains("collection length"));
8383
}
8484
}
8585

8686
#[test]
8787
fn test_map_size_within_limit() {
88-
let fory = Fory::default().max_map_size(3);
88+
let fory = Fory::default().max_collection_size(3);
8989
let mut map: HashMap<String, i32> = HashMap::new();
9090
map.insert("a".to_string(), 1);
9191
map.insert("b".to_string(), 2);
@@ -108,7 +108,7 @@ fn test_btreemap_size_limit_exceeded() {
108108
map.insert("z".to_string(), 3);
109109
let bytes = fory_write.serialize(&map).unwrap();
110110

111-
let fory_read = Fory::default().max_map_size(2);
111+
let fory_read = Fory::default().max_collection_size(2);
112112

113113
if fory_core::error::PANIC_ON_ERROR {
114114
let _ = std::panic::catch_unwind(|| {
@@ -121,13 +121,13 @@ fn test_btreemap_size_limit_exceeded() {
121121
"Expected deserialization to fail due to BTreeMap size limit"
122122
);
123123
let err_msg = format!("{:?}", result.unwrap_err());
124-
assert!(err_msg.contains("map entry count"));
124+
assert!(err_msg.contains("collection length"));
125125
}
126126
}
127127

128128
#[test]
129129
fn test_btreemap_size_within_limit() {
130-
let fory = Fory::default().max_map_size(3);
130+
let fory = Fory::default().max_collection_size(3);
131131
let mut map: BTreeMap<String, i32> = BTreeMap::new();
132132
map.insert("x".to_string(), 1);
133133
map.insert("y".to_string(), 2);
@@ -162,40 +162,6 @@ fn test_hashset_size_limit_exceeded() {
162162
}
163163
}
164164

165-
// ── String ────────────────────────────────────────────────────────────────────
166-
167-
#[test]
168-
fn test_string_size_limit_exceeded() {
169-
let fory_write = Fory::default();
170-
let s = "hello world".to_string();
171-
let bytes = fory_write.serialize(&s).unwrap();
172-
173-
let fory_read = Fory::default().max_string_bytes(5);
174-
175-
if fory_core::error::PANIC_ON_ERROR {
176-
let _ = std::panic::catch_unwind(|| {
177-
let _: Result<String, _> = fory_read.deserialize(&bytes);
178-
});
179-
} else {
180-
let result: Result<String, _> = fory_read.deserialize(&bytes);
181-
assert!(
182-
result.is_err(),
183-
"Expected deserialization to fail due to string size limit"
184-
);
185-
let err_msg = format!("{:?}", result.unwrap_err());
186-
assert!(err_msg.contains("string byte length"));
187-
}
188-
}
189-
190-
#[test]
191-
fn test_string_size_within_limit() {
192-
let fory = Fory::default().max_string_bytes(20);
193-
let s = "hello world".to_string();
194-
let bytes = fory.serialize(&s).unwrap();
195-
let result: Result<String, _> = fory.deserialize(&bytes);
196-
assert!(result.is_ok());
197-
}
198-
199165
// ── Primitive list (Vec<u8>) ──────────────────────────────────────────────────
200166

201167
#[test]
@@ -227,9 +193,9 @@ fn test_primitive_vec_size_limit_exceeded() {
227193

228194
#[test]
229195
fn test_buffer_truncation_rejected() {
230-
// Validates the buffer-remaining cross-check in check_string_bytes
231-
// independently of any configured limit: a structurally truncated buffer
232-
// must be rejected even with default (i32::MAX) limits.
196+
// Validates the buffer-remaining cross-check independently of any
197+
// configured limit: a structurally truncated buffer
198+
// must be rejected even with default limits.
233199
let fory_write = Fory::default();
234200
let s = "hello world".to_string();
235201
let bytes = fory_write.serialize(&s).unwrap();

0 commit comments

Comments
 (0)