Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .spelling
Original file line number Diff line number Diff line change
Expand Up @@ -863,3 +863,4 @@ hardcoding
Idempotence
symlinks
junctions
recurse
7 changes: 7 additions & 0 deletions crates/rest_over_grpc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion crates/rest_over_grpc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand All @@ -218,7 +226,7 @@ as an Axum fallback service.
This crate was developed as part of <a href="https://github.com/microsoft/oxidizer">The Oxidizer Project</a>. Browse this crate's <a href="https://github.com/microsoft/oxidizer/tree/main/crates/rest_over_grpc">source code</a>.
</sub>

[__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
Expand Down
8 changes: 8 additions & 0 deletions crates/rest_over_grpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
54 changes: 52 additions & 2 deletions crates/rest_over_grpc/src/transcode/overlay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,19 @@ 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.
///
/// 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<T: DeserializeOwned>(body: Option<&[u8]>, query: &[(&str, &str)]) -> Result<T, TranscodeError> {
let body = match body {
Some(bytes) if !bytes.is_empty() => match from_slice::<BodyTop<'_>>(bytes) {
Expand All @@ -449,16 +462,26 @@ fn decode_tree_entries<T: DeserializeOwned>(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
Comment thread
Vaiz marked this conversation as resolved.
/// cannot recurse without bound.
fn insert_query_node<'a>(
map: &mut Vec<(String, QueryNode)>,
mut path: impl Iterator<Item = &'a str> + 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"))?;
Expand All @@ -478,7 +501,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 {
Expand Down Expand Up @@ -579,6 +602,8 @@ mod tests {
use serde::Deserialize;

use super::*;
use crate::codegen_helpers::decode_request;
use crate::handling::Code;

#[derive(Debug, Deserialize, PartialEq)]
struct Shelf {
Expand Down Expand Up @@ -733,6 +758,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::<Shelf>(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::<Shelf>(&[(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::<Shelf>(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"}"#;
Expand Down
Loading