From 0fb2527c6095b67b1cd1bce8b5016acb83af3ad3 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 12 Feb 2026 21:28:17 +0900 Subject: [PATCH] fix(router,web): fix panic when normalizing and rewriting paths (#3919) --- actix-router/CHANGES.md | 2 + actix-router/src/path.rs | 49 ++++++++++++++++++ actix-web/CHANGES.md | 2 + actix-web/src/middleware/normalize.rs | 71 ++++++++++++++++++++++++++- 4 files changed, 122 insertions(+), 2 deletions(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index 950880dc8..4c852ba75 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -4,8 +4,10 @@ - Minimum supported Rust version (MSRV) is now 1.88. - Support `deserialize_any` in `PathDeserializer` (enables derived `#[serde(untagged)]` enums in path segments). [#2881] +- Fix stale path segment indices after path rewrites, preventing out-of-bounds access during extraction. [#3562] [#2881]: https://github.com/actix/actix-web/pull/2881 +[#3562]: https://github.com/actix/actix-web/issues/3562 ## 0.5.3 diff --git a/actix-router/src/path.rs b/actix-router/src/path.rs index ab4a943fe..3d1bb27bc 100644 --- a/actix-router/src/path.rs +++ b/actix-router/src/path.rs @@ -93,6 +93,45 @@ impl Path { self.segments.clear(); } + /// Set new path while preserving and remapping existing captured segment indices. + /// + /// The `reindex` closure maps byte indices from the previous path to byte indices in the new + /// path. + #[doc(hidden)] + pub fn update_with_reindex(&mut self, path: T, mut reindex: F) + where + F: FnMut(u16) -> u16, + { + self.skip = reindex(self.skip); + + for (_, item) in &mut self.segments { + if let PathItem::Segment(start, end) = item { + *start = reindex(*start); + *end = reindex(*end); + + if *start > *end { + *start = *end; + } + } + } + + self.path = path; + let path = self.path.path(); + + self.skip = clamp_to_char_boundary(path, self.skip); + + for (_, item) in &mut self.segments { + if let PathItem::Segment(start, end) = item { + *start = clamp_to_char_boundary(path, *start); + *end = clamp_to_char_boundary(path, *end); + + if *start > *end { + *start = *end; + } + } + } + } + /// Reset state. #[inline] pub fn reset(&mut self) { @@ -179,6 +218,16 @@ impl Path { } } +fn clamp_to_char_boundary(path: &str, idx: u16) -> u16 { + let mut idx = usize::from(idx).min(path.len()); + + while idx > 0 && !path.is_char_boundary(idx) { + idx -= 1; + } + + idx as u16 +} + #[derive(Debug)] pub struct PathIter<'a, T> { idx: usize, diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 96f0126d9..bebc1d3eb 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -7,10 +7,12 @@ - Ignore unparsable cookies in `Cookie` request header. - Add `experimental-introspection` feature to report configured routes [#3594] - Add config/method for `TCP_NODELAY`. [#3918] +- Fix panic when `NormalizePath` rewrites a scoped dynamic path before extraction (e.g., `scope("{tail:.*}")` + `Path`). [#3562] [#3895]: https://github.com/actix/actix-web/pull/3895 [#3594]: https://github.com/actix/actix-web/pull/3594 [#3918]: https://github.com/actix/actix-web/pull/3918 +[#3562]: https://github.com/actix/actix-web/issues/3562 ## 4.12.1 diff --git a/actix-web/src/middleware/normalize.rs b/actix-web/src/middleware/normalize.rs index 482107ecb..e98fc4e7a 100644 --- a/actix-web/src/middleware/normalize.rs +++ b/actix-web/src/middleware/normalize.rs @@ -1,6 +1,7 @@ //! For middleware documentation, see [`NormalizePath`]. use actix_http::uri::{PathAndQuery, Uri}; +use actix_router::Url; use actix_service::{Service, Transform}; use actix_utils::future::{ready, Ready}; use bytes::Bytes; @@ -14,6 +15,28 @@ use crate::{ Error, }; +fn build_byte_index_map(old_path: &str, new_path: &str) -> Vec { + let old_path = old_path.as_bytes(); + let new_path = new_path.as_bytes(); + + let mut map = Vec::with_capacity(old_path.len() + 1); + map.push(0); + + let mut old_idx = 0usize; + let mut new_idx = 0usize; + + while old_idx < old_path.len() { + if new_idx < new_path.len() && old_path[old_idx] == new_path[new_idx] { + new_idx += 1; + } + + old_idx += 1; + map.push(new_idx.min(u16::MAX as usize) as u16); + } + + map +} + /// Determines the behavior of the [`NormalizePath`] middleware. /// /// The default is `TrailingSlash::Trim`. @@ -183,6 +206,7 @@ where // Both of the paths have the same length, // so the change can not be deduced from the length comparison if path != original_path { + let reindex = build_byte_index_map(original_path, path); let mut parts = head.uri.clone().into_parts(); let query = parts.path_and_query.as_ref().and_then(|pq| pq.query()); @@ -193,7 +217,11 @@ where parts.path_and_query = Some(PathAndQuery::from_maybe_shared(path).unwrap()); let uri = Uri::from_parts(parts).unwrap(); - req.match_info_mut().get_mut().update(&uri); + req.match_info_mut() + .update_with_reindex(Url::new(uri.clone()), |idx| { + let idx = usize::from(idx).min(reindex.len() - 1); + reindex[idx] + }); req.head_mut().uri = uri; } } @@ -209,7 +237,7 @@ mod tests { use super::*; use crate::{ guard::fn_guard, - test::{call_service, init_service, TestRequest}, + test::{call_service, init_service, read_body, TestRequest}, web, App, HttpResponse, }; @@ -406,6 +434,45 @@ mod tests { } } + #[actix_rt::test] + async fn scope_dynamic_tail_path_is_reindexed() { + async fn handler(path: web::Path) -> HttpResponse { + HttpResponse::Ok().body(path.into_inner()) + } + + let app = init_service( + App::new().service( + web::scope("{tail:.*}") + .wrap(NormalizePath::trim()) + .default_service(web::to(handler)), + ), + ) + .await; + + let req = TestRequest::with_uri("/uaie//iuaei").to_request(); + let res = call_service(&app, req).await; + + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(read_body(res).await, Bytes::from_static(b"uaie/iuaei")); + } + + #[actix_rt::test] + async fn scope_static_prefix_skip_is_reindexed() { + let app = init_service( + App::new().service( + web::scope("/api") + .wrap(NormalizePath::trim()) + .service(web::resource("/v1").to(HttpResponse::Ok)), + ), + ) + .await; + + let req = TestRequest::with_uri("/api//v1").to_request(); + let res = call_service(&app, req).await; + + assert_eq!(res.status(), StatusCode::OK); + } + #[actix_rt::test] async fn no_path() { let app = init_service(