From db93d7a715b327e9a229e666ee95e54ac3b1f73e Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Mon, 21 Sep 2026 16:56:12 +0800 Subject: [PATCH 1/4] Bound wire-declared structural lengths in the decoder (candid 0.10.36) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The type table size already asserts a bound on its wire-declared length, but the sibling counts in the header did not: the argument count (`Header.len`), a record or variant's field count (`Fields.len`), a function type's argument and result counts (`FuncType.arg_len`/`ret_len`), and a service type's method count (`ServType.len`). Each feeds a `#[br(count = len)]`, so an out-of-range value reserved a correspondingly large buffer up front. Cap each of them the way the type table already is: a well-formed message keeps every structural count proportional to the type description, so an out-of-range count now surfaces as an ordinary parse error. The argument count reuses the configurable `max_type_len` limit; the type-table-internal counts use the same default bound as the type table. The wire format is unchanged and valid messages decode identically. Also share the undecoded argument queue behind a reference count. When decoding a present `opt` whose wire and expected types differ, the deserializer snapshots itself to restore on a subtype mismatch. That snapshot only needs the fields a sub-decode can mutate, and the argument queue is not one of them — it is touched only at the top level — so it is now shared via `Rc` rather than cloned, turning the snapshot into a refcount bump. Decoding many optional trailing arguments is correspondingly cheaper and the result is unchanged. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 8 ++++ Cargo.lock | 8 ++-- rust/candid/Cargo.toml | 4 +- rust/candid/src/binary_parser.rs | 11 +++++ rust/candid/src/de.rs | 10 +++-- rust/candid/tests/arg_count_alloc.rs | 60 ++++++++++++++++++++++++++++ rust/candid_derive/Cargo.toml | 2 +- 7 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 rust/candid/tests/arg_count_alloc.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ee12ca4..d21ff025 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 2026-09-21 + +### Candid 0.10.36 + +* Bug fixes: + + Bound the remaining wire-declared structural counts in the type-table header, matching the bound the type table size already carries. The number of arguments, a record or variant's field count, a function type's argument and result counts, and a service type's method count each drive a `count`-sized allocation; a well-formed message keeps every one of them proportional to the type description, so they are now capped the same way. An out-of-range count surfaces as an ordinary parse error instead of reserving a correspondingly large buffer. The argument count reuses the configurable `max_type_len` limit; the type-table-internal counts use the same default bound as the type table. The wire format is unchanged and valid messages decode identically. + + Share the undecoded argument queue behind a reference count so the option/backtracking path no longer copies it. When decoding a present `opt` whose wire and expected types differ, the deserializer takes a snapshot to restore on a subtype mismatch. That snapshot only needs the fields a sub-decode can mutate, and the argument queue is not one of them — it is touched only at the top level — so it is now shared rather than cloned. Decoding many optional trailing arguments is correspondingly cheaper, and the decode result is unchanged. + ## 2026-08-14 ### candid_parser 0.4.1 diff --git a/Cargo.lock b/Cargo.lock index 2c541f48..14e0b7c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,13 +244,13 @@ dependencies = [ [[package]] name = "candid" -version = "0.10.35" +version = "0.10.36" dependencies = [ "anyhow", "bincode", "binrw", "byteorder", - "candid_derive 0.10.35", + "candid_derive 0.10.36", "candid_parser 0.4.1", "hex", "ic_principal 0.1.5", @@ -282,7 +282,7 @@ dependencies = [ [[package]] name = "candid_derive" -version = "0.10.35" +version = "0.10.36" dependencies = [ "lazy_static", "proc-macro2 1.0.86", @@ -315,7 +315,7 @@ version = "0.4.1" dependencies = [ "anyhow", "arbitrary", - "candid 0.10.35", + "candid 0.10.36", "codespan-reporting", "console", "convert_case", diff --git a/rust/candid/Cargo.toml b/rust/candid/Cargo.toml index 71acd779..b18ea717 100644 --- a/rust/candid/Cargo.toml +++ b/rust/candid/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "candid" # sync with the version in `candid_derive/Cargo.toml` -version = "0.10.35" +version = "0.10.36" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] @@ -16,7 +16,7 @@ keywords = ["internet-computer", "idl", "candid", "dfinity"] include = ["src", "Cargo.toml", "LICENSE", "README.md"] [dependencies] -candid_derive = { path = "../candid_derive", version = "=0.10.35" } +candid_derive = { path = "../candid_derive", version = "=0.10.36" } ic_principal = { path = "../ic_principal", version = "0.1.0" } # `verbose-backtrace` (a default feature) pulls in `owo-colors` and generates # backtrace frames that `get_binread_labels` discards, so keep it off. diff --git a/rust/candid/src/binary_parser.rs b/rust/candid/src/binary_parser.rs index 7efbd188..ba31d13e 100644 --- a/rust/candid/src/binary_parser.rs +++ b/rust/candid/src/binary_parser.rs @@ -102,7 +102,12 @@ fn read_leb_usize(name: &'static str, range_msg: &'static str) -> BinResult, @@ -140,7 +145,10 @@ struct IndexType { } #[derive(BinRead, Debug)] struct Fields { + // Each field descriptor drives a `count`-sized allocation; keep the field + // count within the same structural bound as the type table itself. #[br(parse_with = read_leb_u32, args("len", "field length out of 32-bit range"))] + #[br(assert(len as u64 <= MAX_TYPE_TABLE_LEN, "number of fields exceeded"))] len: u32, #[br(count = len)] inner: Vec, @@ -154,10 +162,12 @@ struct FieldType { #[derive(BinRead, Debug)] struct FuncType { #[br(parse_with = read_leb, args("arg_len"))] + #[br(assert(arg_len <= MAX_TYPE_TABLE_LEN, "number of function arguments exceeded"))] arg_len: u64, #[br(count = arg_len)] args: Vec, #[br(parse_with = read_leb, args("ret_len"))] + #[br(assert(ret_len <= MAX_TYPE_TABLE_LEN, "number of function results exceeded"))] ret_len: u64, #[br(count = ret_len)] rets: Vec, @@ -169,6 +179,7 @@ struct FuncType { #[derive(BinRead, Debug)] struct ServType { #[br(parse_with = read_leb, args("len"))] + #[br(assert(len <= MAX_TYPE_TABLE_LEN, "number of service methods exceeded"))] len: u64, #[br(count = len)] meths: Vec, diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index 7c0bc4b3..e8245ee8 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -90,7 +90,7 @@ impl<'de> IDLDeserialize<'de> { } } - let (ind, ty) = self.de.types.pop_front().unwrap(); + let (ind, ty) = Rc::make_mut(&mut self.de.types).pop_front().unwrap(); self.de.expect_type = if matches!(expected_type.as_ref(), TypeInner::Unknown) { self.de.is_untyped = true; ty.clone() @@ -289,7 +289,11 @@ macro_rules! check { struct Deserializer<'de> { input: Cursor<&'de [u8]>, table: Rc, - types: VecDeque<(usize, Type)>, + // The undecoded argument queue. It is only ever mutated at the top level + // (`IDLDeserialize::get_value`), never during a value sub-decode, so holding + // it behind an `Rc` lets the backtracking snapshot in `recoverable_visit_some` + // share it with a refcount bump instead of copying the whole queue. + types: Rc>, wire_type: Type, expect_type: Type, // Memo table for subtyping relation @@ -316,7 +320,7 @@ impl<'de> Deserializer<'de> { Ok(Deserializer { input: reader, table: env.into(), - types: types.into_iter().enumerate().collect(), + types: Rc::new(types.into_iter().enumerate().collect()), wire_type: TypeInner::Unknown.into(), expect_type: TypeInner::Unknown.into(), gamma: Gamma::default(), diff --git a/rust/candid/tests/arg_count_alloc.rs b/rust/candid/tests/arg_count_alloc.rs new file mode 100644 index 00000000..b5d4b277 --- /dev/null +++ b/rust/candid/tests/arg_count_alloc.rs @@ -0,0 +1,60 @@ +//! Wire-declared structural lengths must not turn into oversized up-front +//! allocations. A length prefix need not match the bytes actually present, so an +//! out-of-range argument, field, function-arity or method count must fail as an +//! ordinary parse error rather than reserving a correspondingly large buffer. +//! +//! The type table already carries this bound; these cases cover the remaining +//! length prefixes so that every `count`-sized allocation stays proportional to +//! the type description rather than to the raw byte length. + +use candid::de::IDLDeserialize; +use candid::Encode; + +/// Drive the full deserialize, including skipping every declared argument, so a +/// healthy parser rejects a malformed length and returns normally. +fn decode(hex: &str) -> candid::Result<()> { + let bytes = hex::decode(hex).unwrap(); + let mut de = IDLDeserialize::new(&bytes)?; + while !de.is_done() { + de.get_value::()?; + } + Ok(()) +} + +#[test] +fn argument_count_out_of_range_is_rejected() { + // "DIDL" 00 with an empty type table and no args. + assert!(decode("4449444c00808080808080808040").is_err()); +} + +#[test] +fn record_field_count_out_of_range_is_rejected() { + // "DIDL" 01 6c : a record type declaring 2^62 fields. + assert!(decode("4449444c016c808080808080808040").is_err()); +} + +#[test] +fn function_arity_out_of_range_is_rejected() { + // "DIDL" 01 6a : a func type declaring 2^62 arguments. + assert!(decode("4449444c016a808080808080808040").is_err()); +} + +#[test] +fn service_method_count_out_of_range_is_rejected() { + // "DIDL" 01 69 : a service declaring 2^62 methods. + assert!(decode("4449444c0169808080808080808040").is_err()); +} + +/// A well-formed message with trailing arguments that are skipped through the +/// option/backtracking path still round-trips. This exercises the shared +/// argument queue across the top-level pops that follow a backtracking clone. +#[test] +fn trailing_optional_arguments_still_decode() { + // encode (nat32, opt nat32, opt nat32) and decode only the first value, + // letting `done()` drain the rest through the skip path. + let bytes = Encode!(&7u32, &Some(8u32), &Some(9u32)).unwrap(); + let mut de = IDLDeserialize::new(&bytes).unwrap(); + let first: u32 = de.get_value().unwrap(); + assert_eq!(first, 7); + de.done().unwrap(); +} diff --git a/rust/candid_derive/Cargo.toml b/rust/candid_derive/Cargo.toml index c3ddcd95..3c2f4e78 100644 --- a/rust/candid_derive/Cargo.toml +++ b/rust/candid_derive/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "candid_derive" # sync with the version in `candid/Cargo.toml` -version = "0.10.35" +version = "0.10.36" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] From 49e785fa803516e8b55636f58e3b161481be45e2 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Mon, 21 Sep 2026 17:03:08 +0800 Subject: [PATCH 2/4] Drop version bump; keep fix under an Unreleased changelog section Revert the candid/candid_derive 0.10.36 version bumps (and the lockfile) and replace the versioned changelog header with an Unreleased section. The decoder fix, the regression test, and the changelog bullets are unchanged. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 4 ++-- Cargo.lock | 8 ++++---- rust/candid/Cargo.toml | 4 ++-- rust/candid_derive/Cargo.toml | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d21ff025..aff00fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ # Changelog -## 2026-09-21 +## Unreleased -### Candid 0.10.36 +### Candid * Bug fixes: + Bound the remaining wire-declared structural counts in the type-table header, matching the bound the type table size already carries. The number of arguments, a record or variant's field count, a function type's argument and result counts, and a service type's method count each drive a `count`-sized allocation; a well-formed message keeps every one of them proportional to the type description, so they are now capped the same way. An out-of-range count surfaces as an ordinary parse error instead of reserving a correspondingly large buffer. The argument count reuses the configurable `max_type_len` limit; the type-table-internal counts use the same default bound as the type table. The wire format is unchanged and valid messages decode identically. diff --git a/Cargo.lock b/Cargo.lock index 14e0b7c9..2c541f48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,13 +244,13 @@ dependencies = [ [[package]] name = "candid" -version = "0.10.36" +version = "0.10.35" dependencies = [ "anyhow", "bincode", "binrw", "byteorder", - "candid_derive 0.10.36", + "candid_derive 0.10.35", "candid_parser 0.4.1", "hex", "ic_principal 0.1.5", @@ -282,7 +282,7 @@ dependencies = [ [[package]] name = "candid_derive" -version = "0.10.36" +version = "0.10.35" dependencies = [ "lazy_static", "proc-macro2 1.0.86", @@ -315,7 +315,7 @@ version = "0.4.1" dependencies = [ "anyhow", "arbitrary", - "candid 0.10.36", + "candid 0.10.35", "codespan-reporting", "console", "convert_case", diff --git a/rust/candid/Cargo.toml b/rust/candid/Cargo.toml index b18ea717..71acd779 100644 --- a/rust/candid/Cargo.toml +++ b/rust/candid/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "candid" # sync with the version in `candid_derive/Cargo.toml` -version = "0.10.36" +version = "0.10.35" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] @@ -16,7 +16,7 @@ keywords = ["internet-computer", "idl", "candid", "dfinity"] include = ["src", "Cargo.toml", "LICENSE", "README.md"] [dependencies] -candid_derive = { path = "../candid_derive", version = "=0.10.36" } +candid_derive = { path = "../candid_derive", version = "=0.10.35" } ic_principal = { path = "../ic_principal", version = "0.1.0" } # `verbose-backtrace` (a default feature) pulls in `owo-colors` and generates # backtrace frames that `get_binread_labels` discards, so keep it off. diff --git a/rust/candid_derive/Cargo.toml b/rust/candid_derive/Cargo.toml index 3c2f4e78..c3ddcd95 100644 --- a/rust/candid_derive/Cargo.toml +++ b/rust/candid_derive/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "candid_derive" # sync with the version in `candid/Cargo.toml` -version = "0.10.36" +version = "0.10.35" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] From 6df997724503870ed77b0151eeae177d7f51289e Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Mon, 21 Sep 2026 17:12:40 +0800 Subject: [PATCH 3/4] Address review: doc max_type_len arg bound, strengthen decoder tests - Document that `set_max_type_len` also bounds the top-level argument count, with the `set_max_type_len(1)` example, and add a config test asserting it rejects an over-limit argument count and accepts one within the limit. - Use a u32-fitting field count (2^31) in the record-field test so parsing reaches the new field-count bound instead of failing the u32 conversion first. - Add a function result-count test (zero args, oversized result count) so the `ret_len` bound has independent coverage. - Add a test that a present `opt` with a mismatched inner type takes the backtracking arm, decodes as `None`, and leaves the shared argument queue intact for the following argument. Co-Authored-By: Claude Opus 4.8 --- rust/candid/src/de.rs | 6 +++- rust/candid/tests/arg_count_alloc.rs | 47 ++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index e8245ee8..4a476736 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -217,7 +217,11 @@ impl DecoderConfig { self.skipping_quota = Some(n); self } - /// Set the max type table size + /// Set the max type table size. This also bounds the number of top-level + /// arguments a message may declare, since a well-formed message declares an + /// argument per value it carries and that count stays proportional to the + /// type table describing them. For example, `set_max_type_len(1)` rejects a + /// message declaring two arguments even when its type table is empty. pub fn set_max_type_len(&mut self, n: usize) -> &mut Self { self.max_type_len = Some(n); self diff --git a/rust/candid/tests/arg_count_alloc.rs b/rust/candid/tests/arg_count_alloc.rs index b5d4b277..462583bc 100644 --- a/rust/candid/tests/arg_count_alloc.rs +++ b/rust/candid/tests/arg_count_alloc.rs @@ -8,7 +8,7 @@ //! the type description rather than to the raw byte length. use candid::de::IDLDeserialize; -use candid::Encode; +use candid::{DecoderConfig, Encode}; /// Drive the full deserialize, including skipping every declared argument, so a /// healthy parser rejects a malformed length and returns normally. @@ -29,8 +29,10 @@ fn argument_count_out_of_range_is_rejected() { #[test] fn record_field_count_out_of_range_is_rejected() { - // "DIDL" 01 6c : a record type declaring 2^62 fields. - assert!(decode("4449444c016c808080808080808040").is_err()); + // "DIDL" 01 6c : a record type declaring 2^31 + // fields. The field count is a u32 on the wire, so the value is chosen to + // fit in u32 and reach the new bound rather than failing the u32 conversion. + assert!(decode("4449444c016c8080808008").is_err()); } #[test] @@ -39,6 +41,13 @@ fn function_arity_out_of_range_is_rejected() { assert!(decode("4449444c016a808080808080808040").is_err()); } +#[test] +fn function_result_count_out_of_range_is_rejected() { + // "DIDL" 01 6a 00 : a func type with zero arguments + // and 2^62 results, so the independent result-count bound is exercised. + assert!(decode("4449444c016a00808080808080808040").is_err()); +} + #[test] fn service_method_count_out_of_range_is_rejected() { // "DIDL" 01 69 : a service declaring 2^62 methods. @@ -58,3 +67,35 @@ fn trailing_optional_arguments_still_decode() { assert_eq!(first, 7); de.done().unwrap(); } + +/// A present `opt` whose inner wire type is not a subtype of the expected inner +/// type takes the backtracking arm: `recoverable_visit_some` restores the +/// snapshot and decodes the value as `None`. The following argument must still +/// decode, proving the shared argument queue survives the restore intact. +#[test] +fn mismatched_optional_restores_shared_queue() { + // wire: (opt text = ?"x", nat32 = 42); decode the first into Option. + let bytes = Encode!(&Some("x".to_string()), &42u32).unwrap(); + let mut de = IDLDeserialize::new(&bytes).unwrap(); + let first: Option = de.get_value().unwrap(); + assert_eq!(first, None); + let second: u32 = de.get_value().unwrap(); + assert_eq!(second, 42); + de.done().unwrap(); +} + +/// `max_type_len` bounds the top-level argument count in addition to the type +/// table size, so a message declaring more arguments than the limit is rejected +/// while one within the limit still decodes. +#[test] +fn max_type_len_bounds_argument_count() { + let two_args = Encode!(&1u32, &2u32).unwrap(); + + let mut too_small = DecoderConfig::new(); + too_small.set_max_type_len(1); + assert!(IDLDeserialize::new_with_config(&two_args, &too_small).is_err()); + + let mut big_enough = DecoderConfig::new(); + big_enough.set_max_type_len(2); + assert!(IDLDeserialize::new_with_config(&two_args, &big_enough).is_ok()); +} From 8d3774796c6a893198ab72134d9accc47697ba0b Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Mon, 21 Sep 2026 18:01:48 +0800 Subject: [PATCH 4/4] Narrow to the argument-queue sharing fix; drop the structural-count caps Review measurement showed the count bounds this branch added were both unnecessary and a compatibility regression, and the shared-queue change is the actual fix: - binrw's `count` only pre-reserves for `Vec` (already handled by `read_len_prefixed`); for the header's element vectors it collects a `Result` iterator whose size hint lower bound is 0, so no oversized buffer was ever reserved. The malformed-count inputs already fail on the short read, so the asserts fixed no defect. - The 10_000 caps rejected messages that are valid today and that decode fine on master (for example a record with 10_001 null fields, a 30 KB message), and reusing `max_type_len` for the argument count silently changed a public config API. Both are removed. - Sharing the undecoded argument queue behind an `Rc` is what removes the superlinear cost: skipping present optional arguments went from O(args x present-opts) to linear in the argument count. The new regression test exercises this directly and trips only if the per-opt copy returns. Restores `binary_parser.rs` and the `set_max_type_len` doc to master, keeps the `de.rs` queue-sharing change, and replaces the count tests with `skip_optional_queue.rs`. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 3 +- rust/candid/src/binary_parser.rs | 11 --- rust/candid/src/de.rs | 6 +- rust/candid/tests/arg_count_alloc.rs | 101 ----------------------- rust/candid/tests/skip_optional_queue.rs | 81 ++++++++++++++++++ 5 files changed, 83 insertions(+), 119 deletions(-) delete mode 100644 rust/candid/tests/arg_count_alloc.rs create mode 100644 rust/candid/tests/skip_optional_queue.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index aff00fe4..5cf53a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,7 @@ ### Candid * Bug fixes: - + Bound the remaining wire-declared structural counts in the type-table header, matching the bound the type table size already carries. The number of arguments, a record or variant's field count, a function type's argument and result counts, and a service type's method count each drive a `count`-sized allocation; a well-formed message keeps every one of them proportional to the type description, so they are now capped the same way. An out-of-range count surfaces as an ordinary parse error instead of reserving a correspondingly large buffer. The argument count reuses the configurable `max_type_len` limit; the type-table-internal counts use the same default bound as the type table. The wire format is unchanged and valid messages decode identically. - + Share the undecoded argument queue behind a reference count so the option/backtracking path no longer copies it. When decoding a present `opt` whose wire and expected types differ, the deserializer takes a snapshot to restore on a subtype mismatch. That snapshot only needs the fields a sub-decode can mutate, and the argument queue is not one of them — it is touched only at the top level — so it is now shared rather than cloned. Decoding many optional trailing arguments is correspondingly cheaper, and the decode result is unchanged. + + Share the undecoded argument queue behind a reference count. The option/backtracking path snapshots the deserializer to restore on a subtype mismatch, which previously copied the whole queue of remaining arguments on every present `opt`. The queue is only mutated at the top level, so it is now shared rather than copied and the snapshot is a refcount bump. Skipping many present optional arguments is no longer superlinear in the argument count; the decode result is unchanged. ## 2026-08-14 diff --git a/rust/candid/src/binary_parser.rs b/rust/candid/src/binary_parser.rs index ba31d13e..7efbd188 100644 --- a/rust/candid/src/binary_parser.rs +++ b/rust/candid/src/binary_parser.rs @@ -102,12 +102,7 @@ fn read_leb_usize(name: &'static str, range_msg: &'static str) -> BinResult, @@ -145,10 +140,7 @@ struct IndexType { } #[derive(BinRead, Debug)] struct Fields { - // Each field descriptor drives a `count`-sized allocation; keep the field - // count within the same structural bound as the type table itself. #[br(parse_with = read_leb_u32, args("len", "field length out of 32-bit range"))] - #[br(assert(len as u64 <= MAX_TYPE_TABLE_LEN, "number of fields exceeded"))] len: u32, #[br(count = len)] inner: Vec, @@ -162,12 +154,10 @@ struct FieldType { #[derive(BinRead, Debug)] struct FuncType { #[br(parse_with = read_leb, args("arg_len"))] - #[br(assert(arg_len <= MAX_TYPE_TABLE_LEN, "number of function arguments exceeded"))] arg_len: u64, #[br(count = arg_len)] args: Vec, #[br(parse_with = read_leb, args("ret_len"))] - #[br(assert(ret_len <= MAX_TYPE_TABLE_LEN, "number of function results exceeded"))] ret_len: u64, #[br(count = ret_len)] rets: Vec, @@ -179,7 +169,6 @@ struct FuncType { #[derive(BinRead, Debug)] struct ServType { #[br(parse_with = read_leb, args("len"))] - #[br(assert(len <= MAX_TYPE_TABLE_LEN, "number of service methods exceeded"))] len: u64, #[br(count = len)] meths: Vec, diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index 4a476736..e8245ee8 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -217,11 +217,7 @@ impl DecoderConfig { self.skipping_quota = Some(n); self } - /// Set the max type table size. This also bounds the number of top-level - /// arguments a message may declare, since a well-formed message declares an - /// argument per value it carries and that count stays proportional to the - /// type table describing them. For example, `set_max_type_len(1)` rejects a - /// message declaring two arguments even when its type table is empty. + /// Set the max type table size pub fn set_max_type_len(&mut self, n: usize) -> &mut Self { self.max_type_len = Some(n); self diff --git a/rust/candid/tests/arg_count_alloc.rs b/rust/candid/tests/arg_count_alloc.rs deleted file mode 100644 index 462583bc..00000000 --- a/rust/candid/tests/arg_count_alloc.rs +++ /dev/null @@ -1,101 +0,0 @@ -//! Wire-declared structural lengths must not turn into oversized up-front -//! allocations. A length prefix need not match the bytes actually present, so an -//! out-of-range argument, field, function-arity or method count must fail as an -//! ordinary parse error rather than reserving a correspondingly large buffer. -//! -//! The type table already carries this bound; these cases cover the remaining -//! length prefixes so that every `count`-sized allocation stays proportional to -//! the type description rather than to the raw byte length. - -use candid::de::IDLDeserialize; -use candid::{DecoderConfig, Encode}; - -/// Drive the full deserialize, including skipping every declared argument, so a -/// healthy parser rejects a malformed length and returns normally. -fn decode(hex: &str) -> candid::Result<()> { - let bytes = hex::decode(hex).unwrap(); - let mut de = IDLDeserialize::new(&bytes)?; - while !de.is_done() { - de.get_value::()?; - } - Ok(()) -} - -#[test] -fn argument_count_out_of_range_is_rejected() { - // "DIDL" 00 with an empty type table and no args. - assert!(decode("4449444c00808080808080808040").is_err()); -} - -#[test] -fn record_field_count_out_of_range_is_rejected() { - // "DIDL" 01 6c : a record type declaring 2^31 - // fields. The field count is a u32 on the wire, so the value is chosen to - // fit in u32 and reach the new bound rather than failing the u32 conversion. - assert!(decode("4449444c016c8080808008").is_err()); -} - -#[test] -fn function_arity_out_of_range_is_rejected() { - // "DIDL" 01 6a : a func type declaring 2^62 arguments. - assert!(decode("4449444c016a808080808080808040").is_err()); -} - -#[test] -fn function_result_count_out_of_range_is_rejected() { - // "DIDL" 01 6a 00 : a func type with zero arguments - // and 2^62 results, so the independent result-count bound is exercised. - assert!(decode("4449444c016a00808080808080808040").is_err()); -} - -#[test] -fn service_method_count_out_of_range_is_rejected() { - // "DIDL" 01 69 : a service declaring 2^62 methods. - assert!(decode("4449444c0169808080808080808040").is_err()); -} - -/// A well-formed message with trailing arguments that are skipped through the -/// option/backtracking path still round-trips. This exercises the shared -/// argument queue across the top-level pops that follow a backtracking clone. -#[test] -fn trailing_optional_arguments_still_decode() { - // encode (nat32, opt nat32, opt nat32) and decode only the first value, - // letting `done()` drain the rest through the skip path. - let bytes = Encode!(&7u32, &Some(8u32), &Some(9u32)).unwrap(); - let mut de = IDLDeserialize::new(&bytes).unwrap(); - let first: u32 = de.get_value().unwrap(); - assert_eq!(first, 7); - de.done().unwrap(); -} - -/// A present `opt` whose inner wire type is not a subtype of the expected inner -/// type takes the backtracking arm: `recoverable_visit_some` restores the -/// snapshot and decodes the value as `None`. The following argument must still -/// decode, proving the shared argument queue survives the restore intact. -#[test] -fn mismatched_optional_restores_shared_queue() { - // wire: (opt text = ?"x", nat32 = 42); decode the first into Option. - let bytes = Encode!(&Some("x".to_string()), &42u32).unwrap(); - let mut de = IDLDeserialize::new(&bytes).unwrap(); - let first: Option = de.get_value().unwrap(); - assert_eq!(first, None); - let second: u32 = de.get_value().unwrap(); - assert_eq!(second, 42); - de.done().unwrap(); -} - -/// `max_type_len` bounds the top-level argument count in addition to the type -/// table size, so a message declaring more arguments than the limit is rejected -/// while one within the limit still decodes. -#[test] -fn max_type_len_bounds_argument_count() { - let two_args = Encode!(&1u32, &2u32).unwrap(); - - let mut too_small = DecoderConfig::new(); - too_small.set_max_type_len(1); - assert!(IDLDeserialize::new_with_config(&two_args, &too_small).is_err()); - - let mut big_enough = DecoderConfig::new(); - big_enough.set_max_type_len(2); - assert!(IDLDeserialize::new_with_config(&two_args, &big_enough).is_ok()); -} diff --git a/rust/candid/tests/skip_optional_queue.rs b/rust/candid/tests/skip_optional_queue.rs new file mode 100644 index 00000000..41eacafe --- /dev/null +++ b/rust/candid/tests/skip_optional_queue.rs @@ -0,0 +1,81 @@ +//! The deserializer shares its undecoded argument queue behind a reference +//! count, so the option/backtracking path no longer copies it. These tests lock +//! the correctness of that shared queue across the top-level pops that follow a +//! backtracking snapshot, and guard against reintroducing a per-`opt` copy whose +//! cost grows with the number of remaining arguments. + +use candid::de::IDLDeserialize; +use candid::Encode; +use std::time::{Duration, Instant}; + +/// A well-formed message with trailing arguments that are skipped through the +/// option path still round-trips. Skipping each present `opt` takes the snapshot +/// arm, so this exercises the shared queue across the pops that follow it. +#[test] +fn trailing_optional_arguments_still_decode() { + // encode (nat32, opt nat32, opt nat32) and decode only the first value, + // letting `done()` drain the rest through the skip path. + let bytes = Encode!(&7u32, &Some(8u32), &Some(9u32)).unwrap(); + let mut de = IDLDeserialize::new(&bytes).unwrap(); + let first: u32 = de.get_value().unwrap(); + assert_eq!(first, 7); + de.done().unwrap(); +} + +/// A present `opt` whose inner wire type is not a subtype of the expected inner +/// type takes the backtracking arm: `recoverable_visit_some` restores the +/// snapshot and decodes the value as `None`. The following argument must still +/// decode, proving the shared argument queue survives the restore intact. +#[test] +fn mismatched_optional_restores_shared_queue() { + // wire: (opt text = ?"x", nat32 = 42); decode the first into Option. + let bytes = Encode!(&Some("x".to_string()), &42u32).unwrap(); + let mut de = IDLDeserialize::new(&bytes).unwrap(); + let first: Option = de.get_value().unwrap(); + assert_eq!(first, None); + let second: u32 = de.get_value().unwrap(); + assert_eq!(second, 42); + de.done().unwrap(); +} + +/// Build a message declaring `n` arguments of type `opt null`, whose first +/// `present` values are `?null` and the rest `null`. Decoding argument 0 and +/// skipping the remainder walks every present `opt` through the snapshot arm. +fn opt_null_message(n: usize, present: usize) -> Vec { + let mut b = vec![0x44, 0x49, 0x44, 0x4c]; // "DIDL" + b.extend_from_slice(&[0x01, 0x6e, 0x7f]); // type table: opt (0x6e) of null (0x7f) + let mut count = n as u64; // argument count as LEB128 + loop { + let mut byte = (count & 0x7f) as u8; + count >>= 7; + if count != 0 { + byte |= 0x80; + } + b.push(byte); + if count == 0 { + break; + } + } + b.extend(std::iter::repeat(0x00).take(n)); // each argument references table index 0 + b.extend((0..n).map(|i| if i < present { 0x01 } else { 0x00 })); // present/absent flags + b +} + +/// Skipping many present optional arguments must stay proportional to the number +/// of arguments, not to arguments times present-opts. When the snapshot copies +/// the whole remaining queue, this is quadratic and takes many seconds; sharing +/// the queue keeps it in the millisecond range. The threshold is deliberately +/// loose so it only trips on a return of the quadratic behavior. +#[test] +fn skipping_many_present_optionals_is_not_quadratic() { + let bytes = opt_null_message(150_000, 15_000); + let start = Instant::now(); + let mut de = IDLDeserialize::new(&bytes).unwrap(); + let _first: Option<()> = de.get_value().unwrap(); + de.done().unwrap(); + let elapsed = start.elapsed(); + assert!( + elapsed < Duration::from_secs(2), + "skipping present optionals took {elapsed:?}; the argument queue is likely being copied per opt again" + ); +}