From c4d9ec4459a37725f8d367bd0da2c086c6eba9a5 Mon Sep 17 00:00:00 2001 From: Vaiz <4908982+Vaiz@users.noreply.github.com> Date: Fri, 4 Sep 2026 07:48:59 +0100 Subject: [PATCH 1/4] fix(rest_over_grpc): bound query field path nesting depth Dotted query keys built one nesting level per `.`-separated segment with no bound, so the build and the matching deserialization both descended once per segment. Cap the depth at 64 levels and reject a deeper path with `Code::InvalidArgument`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/rest_over_grpc/CHANGELOG.md | 7 +++ .../rest_over_grpc/src/transcode/overlay.rs | 49 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/crates/rest_over_grpc/CHANGELOG.md b/crates/rest_over_grpc/CHANGELOG.md index c379a15a9..a58e3b6ce 100644 --- a/crates/rest_over_grpc/CHANGELOG.md +++ b/crates/rest_over_grpc/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Query parameter field paths are now limited to 64 levels of nesting. A deeper + path is rejected with `Code::InvalidArgument` instead of being decoded. + ## [0.2.0] - 2026-07-24 ### Added diff --git a/crates/rest_over_grpc/src/transcode/overlay.rs b/crates/rest_over_grpc/src/transcode/overlay.rs index 90fcfb6b0..058686d31 100644 --- a/crates/rest_over_grpc/src/transcode/overlay.rs +++ b/crates/rest_over_grpc/src/transcode/overlay.rs @@ -433,6 +433,14 @@ enum QueryNode { Leaf(SmallVec<[String; 2]>), } +/// The deepest `.`-separated field path a query parameter may name. +/// +/// A dotted query key builds one [`QueryNode`] level per segment, and both that +/// build and the matching deserialization descend once per level. The bound +/// keeps an untrusted request from driving that descent past the stack, while +/// staying far above the nesting any real proto message uses. +const MAX_QUERY_FIELD_DEPTH: usize = 64; + fn decode_tree(body: Option<&[u8]>, query: &[(&str, &str)]) -> Result { let body = match body { Some(bytes) if !bytes.is_empty() => match from_slice::>(bytes) { @@ -449,16 +457,26 @@ fn decode_tree_entries(body: Vec<(String, &RawValue)>, quer for (key, value) in query { let key = percent::decode_query(key).ok_or_else(|| TranscodeError::invalid_encoding("query parameter name"))?; let value = percent::decode_query(value).ok_or_else(|| TranscodeError::invalid_encoding("query parameter value"))?; - insert_query_node(&mut query_root, key.split('.'), value.into_owned())?; + insert_query_node(&mut query_root, key.split('.'), value.into_owned(), MAX_QUERY_FIELD_DEPTH)?; } T::deserialize(TreeDeserializer { body, query: query_root }).map_err(TranscodeError::deserialize) } +/// Inserts `value` at the `.`-separated field `path`, descending one level per +/// segment. +/// +/// `remaining_depth` is the number of levels still allowed; it starts at +/// [`MAX_QUERY_FIELD_DEPTH`] and is spent one per segment so an untrusted key +/// cannot recurse without bound. fn insert_query_node<'a>( map: &mut Vec<(String, QueryNode)>, mut path: impl Iterator + Clone, value: String, + remaining_depth: usize, ) -> Result<(), TranscodeError> { + let Some(remaining_depth) = remaining_depth.checked_sub(1) else { + return Err(TranscodeError::structure("query parameter field path is nested too deeply")); + }; let key = path .next() .ok_or_else(|| TranscodeError::structure("query parameter has an empty field path"))?; @@ -478,7 +496,7 @@ fn insert_query_node<'a>( }; if has_more { match node { - QueryNode::Map(children) => insert_query_node(children, path, value)?, + QueryNode::Map(children) => insert_query_node(children, path, value, remaining_depth)?, QueryNode::Leaf(_) => return Err(TranscodeError::structure("query field conflicts with a nested field")), } } else { @@ -579,6 +597,8 @@ mod tests { use serde::Deserialize; use super::*; + use crate::codegen_helpers::decode_request; + use crate::handling::Code; #[derive(Debug, Deserialize, PartialEq)] struct Shelf { @@ -733,6 +753,31 @@ mod tests { assert!(field_mapped.is_none(), "body-mapped field must fall back to the value path"); } + #[test] + fn a_deeply_nested_query_field_path_is_rejected_instead_of_recursing() { + // A key deeper than the bound used to recurse once per segment until the + // stack overflowed and aborted the process. + let key = vec!["a"; MAX_QUERY_FIELD_DEPTH + 1].join("."); + let error = decode_tree::(None, &[(key.as_str(), "1")]).expect_err("over-deep field path is rejected"); + assert_eq!(error.code(), Code::InvalidArgument); + assert!(error.to_string().contains("nested too deeply")); + + // Far past the bound, and through the public entry point, so the guard + // covers the serving path rather than just this helper. + let key = vec!["a"; 100_000].join("."); + let error = + decode_request::(&[(key.as_str(), "1")], b"", RequestBodyKind::None).expect_err("over-deep field path is rejected"); + assert_eq!(error.code(), Code::InvalidArgument); + } + + #[test] + fn a_field_path_at_the_depth_bound_is_still_accepted() { + let key = vec!["a"; MAX_QUERY_FIELD_DEPTH].join("."); + // Rejected for naming an unknown field, not for its depth. + let error = decode_tree::(None, &[(key.as_str(), "1")]).expect_err("unknown nested field"); + assert!(!error.to_string().contains("nested too deeply")); + } + #[test] fn a_repeated_body_key_is_rejected() { let body = br#"{"theme":"first","shelf":"7","theme":"second"}"#; From c3c9a0df4673b5c7926c3428ebd7e08df7a39546 Mon Sep 17 00:00:00 2001 From: Vaiz <4908982+Vaiz@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:54:31 +0100 Subject: [PATCH 2/4] fix(rest_over_grpc): spell "recurse" and document the depth cap as a known limitation Add "recurse" to the repo word list so cargo-spellcheck accepts the sentence that explains why the query field path depth is bounded, and state plainly on MAX_QUERY_FIELD_DEPTH that an over-deep path is rejected rather than truncated, and that the cap is deliberately fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .spelling | 1 + crates/rest_over_grpc/src/transcode/overlay.rs | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/.spelling b/.spelling index ad10d9334..c815d6a7c 100644 --- a/.spelling +++ b/.spelling @@ -863,3 +863,4 @@ hardcoding Idempotence symlinks junctions +recurse diff --git a/crates/rest_over_grpc/src/transcode/overlay.rs b/crates/rest_over_grpc/src/transcode/overlay.rs index 058686d31..94a841b28 100644 --- a/crates/rest_over_grpc/src/transcode/overlay.rs +++ b/crates/rest_over_grpc/src/transcode/overlay.rs @@ -439,6 +439,11 @@ enum QueryNode { /// build and the matching deserialization descend once per level. The bound /// keeps an untrusted request from driving that descent past the stack, while /// staying far above the nesting any real proto message uses. +/// +/// Known limitation: a query parameter whose field path nests deeper than this +/// is rejected with `Code::InvalidArgument`, never truncated and never decoded. +/// The cap is fixed and deliberately not configurable — a depth a caller could +/// raise would not bound anything. const MAX_QUERY_FIELD_DEPTH: usize = 64; fn decode_tree(body: Option<&[u8]>, query: &[(&str, &str)]) -> Result { From 5389189c4334b9744775af7655a302ebc535340e Mon Sep 17 00:00:00 2001 From: Vaiz <4908982+Vaiz@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:42:12 +0100 Subject: [PATCH 3/4] docs(rest_over_grpc): document the query depth limit in the public docs The cap was documented only on the private MAX_QUERY_FIELD_DEPTH constant in a private module, so it never reached rustdoc or the README. State it in the crate-level Limitations section, where a user of the crate can see it, and regenerate the README paragraph to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/rest_over_grpc/README.md | 8 ++++++++ crates/rest_over_grpc/src/lib.rs | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/crates/rest_over_grpc/README.md b/crates/rest_over_grpc/README.md index df56d8725..d50ae2c4d 100644 --- a/crates/rest_over_grpc/README.md +++ b/crates/rest_over_grpc/README.md @@ -199,6 +199,14 @@ and bidirectional RPCs have no `google.api.http` mapping and are rejected by Requests are buffered and parsed as JSON, so there is no incremental request body path and binary payloads must fit JSON-friendly encoding. +Query parameter field paths are limited to 64 levels of nesting. A dotted +key such as `?a.b.c=1` builds one level per segment, so a path deeper than +the limit is rejected as an invalid request rather than decoded, truncated, +or allowed to exhaust the stack. The limit is fixed and not configurable: it +bounds the recursion an untrusted request can drive, and a bound a caller +could raise would not bound anything. It stays far above the nesting any +real proto message uses. + ## Cargo features * `serving` (default): [`serve_http`][__link29], [`serve_http_fn`][__link30], and [`RestBody`][__link31]. diff --git a/crates/rest_over_grpc/src/lib.rs b/crates/rest_over_grpc/src/lib.rs index c044aca06..b065a1703 100644 --- a/crates/rest_over_grpc/src/lib.rs +++ b/crates/rest_over_grpc/src/lib.rs @@ -197,6 +197,14 @@ //! Requests are buffered and parsed as JSON, so there is no incremental request //! body path and binary payloads must fit JSON-friendly encoding. //! +//! Query parameter field paths are limited to 64 levels of nesting. A dotted +//! key such as `?a.b.c=1` builds one level per segment, so a path deeper than +//! the limit is rejected as an invalid request rather than decoded, truncated, +//! or allowed to exhaust the stack. The limit is fixed and not configurable: it +//! bounds the recursion an untrusted request can drive, and a bound a caller +//! could raise would not bound anything. It stays far above the nesting any +//! real proto message uses. +//! //! # Cargo features //! //! - `serving` (default): [`serve_http`](serving::serve_http), [`serve_http_fn`](serving::serve_http_fn), and [`RestBody`](serving::RestBody). From e66d2bd0e5246fd5a69be60bb86ad532ce30e537 Mon Sep 17 00:00:00 2001 From: Vaiz <4908982+Vaiz@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:24:57 +0100 Subject: [PATCH 4/4] docs(rest_over_grpc): refresh the README doc2readme input hash anvil-readme-check regenerates the README with cargo-doc2readme pinned to =0.6.4 and compares the input hash recorded in the dependencies_info blob. The previous commit added the prose by hand, leaving the stale hash, so CI reported "Input has changed". Regenerated with 0.6.4; only the blob moves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/rest_over_grpc/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rest_over_grpc/README.md b/crates/rest_over_grpc/README.md index d50ae2c4d..29b20cef4 100644 --- a/crates/rest_over_grpc/README.md +++ b/crates/rest_over_grpc/README.md @@ -226,7 +226,7 @@ as an Axum fallback service. This crate was developed as part of The Oxidizer Project. Browse this crate's source code. - [__cargo_doc2readme_dependencies_info]: ggGmYW0CYXZlMC43LjJhdIQb11VxC_uAPOQbtUn4Wx2-BfAbid3Nt1Y27Pobprn8Z6FjFy9hYvRhcoQb4yyDbhLmywUbUgoeDyjY0hYb_gBd7xtnrJEbm_ruDQCrgu9hZIOCZ2xheWVyZWRlMC4zLjaCbnJlc3Rfb3Zlcl9ncnBjZTAuMi4wgm10b3dlcl9zZXJ2aWNlZTAuMy4z + [__cargo_doc2readme_dependencies_info]: ggGkYW0CYXSEG9dVcQv7gDzkG7VJ-FsdvgXwG4ndzbdWNuz6G6a5_GehYxcvYXKEG705H0ZGUhe-GxZIkdl02LFVGzaVf60fYr6-G0KoiaJwaM4fYWSDgmdsYXllcmVkZTAuMy42gm5yZXN0X292ZXJfZ3JwY2UwLjIuMIJtdG93ZXJfc2VydmljZWUwLjMuMw [__link0]: https://docs.rs/rest_over_grpc/0.2.0/rest_over_grpc/?search=handling::Status [__link1]: https://docs.rs/rest_over_grpc/0.2.0/rest_over_grpc/?search=serving::RestService::new [__link10]: https://docs.rs/rest_over_grpc/0.2.0/rest_over_grpc/?search=transcoding::Transcode::try_transcode