diff --git a/cspell.json b/cspell.json index 58ecda7b..cd80c79a 100644 --- a/cspell.json +++ b/cspell.json @@ -248,6 +248,7 @@ "guiddef", "handleapi", "hdr", + "hexdigit", "HEXINT", "hklm", "hlist", @@ -374,6 +375,7 @@ "npidof", "nprintf", "NSG", + "nstance", "nsudo", "ntdll", "ntimeout", diff --git a/proxy_agent/src/proxy/proxy_connection.rs b/proxy_agent/src/proxy/proxy_connection.rs index f6867b43..7c6cbae3 100644 --- a/proxy_agent/src/proxy/proxy_connection.rs +++ b/proxy_agent/src/proxy/proxy_connection.rs @@ -303,6 +303,12 @@ impl HttpConnectionContext { path_has_traversal(self.url.path()) } + /// Checks if the request path contains nested percent-encoding, which can be used to bypass + /// certain security checks. Returns `true` if nested percent-encoding is detected. + pub fn has_nested_percent_encoding(&self) -> bool { + path_has_nested_percent_encoding(self.url.path()) + } + pub fn log(&mut self, logger_level: LoggerLevel, message: String) { if config::get_enable_http_proxy_trace() { self.add_stage(message.clone()); @@ -448,9 +454,22 @@ fn path_has_traversal(raw_path: &str) -> bool { decoded.contains("..") } +/// Detect nested percent-encoding without changing request handling. +/// Every deeper encoding exposes a valid `%HH` sequence after one decode. +fn path_has_nested_percent_encoding(raw_path: &str) -> bool { + let decoded = percent_encoding::percent_decode_str(raw_path).decode_utf8_lossy(); + contains_percent_encoded_byte(decoded.as_bytes()) +} + +fn contains_percent_encoded_byte(value: &[u8]) -> bool { + value.windows(3).any(|window| { + window[0] == b'%' && window[1].is_ascii_hexdigit() && window[2].is_ascii_hexdigit() + }) +} + #[cfg(test)] mod tests { - use super::path_has_traversal; + use super::{path_has_nested_percent_encoding, path_has_traversal}; #[test] fn clean_paths_are_not_traversal() { @@ -532,4 +551,33 @@ mod tests { ); } } + + #[test] + fn nested_percent_encoding_tests() { + for p in [ + "/foo/%252e%252e/bar", + "/foo/%252E%252e/bar", + "/foo/%25252e%25252e/bar", + "/foo/%25%32%65%25%32%65/bar", + "/metadata%252fidentity/oauth2/token", + "/metadata/%253Fapi-version=2021-02-01", + ] { + assert!( + path_has_nested_percent_encoding(p), + "expected nested percent encoding to be detected: {p:?}" + ); + } + + for p in [ + "/metadata/%69nstance", + "/metadata/value%25done", + "/metadata/value%252", + "/metadata/value%25zz", + ] { + assert!( + !path_has_nested_percent_encoding(p), + "expected unambiguous encoding to be allowed: {p:?}" + ); + } + } } diff --git a/proxy_agent/src/proxy/proxy_server.rs b/proxy_agent/src/proxy/proxy_server.rs index d9a64277..6580b300 100644 --- a/proxy_agent/src/proxy/proxy_server.rs +++ b/proxy_agent/src/proxy/proxy_server.rs @@ -439,6 +439,18 @@ impl ProxyServer { return Ok(Self::method_not_allowed_response()); } + if http_connection_context.has_nested_percent_encoding() { + // If the proxied request contains nested percent encoding characters, we will return 404 Not Found to avoid potential security issues. + self.log_connection_summary( + &mut http_connection_context, + StatusCode::NOT_FOUND, + false, + "Nested percent encoding found in the request, return NOT FOUND!".to_string(), + ) + .await; + return Ok(Self::closed_response(StatusCode::NOT_FOUND)); + } + if http_connection_context.contains_traversal_characters() { // If the proxied request contains traversal characters, we will return 403 Forbidden to avoid potential security issues. self.log_connection_summary( @@ -1330,6 +1342,39 @@ mod tests { "response.status must be FORBIDDEN." ); + // Match IIS request-filtering behavior for double and deeper + // percent encoding without recursively decoding the request. + for path in [ + "/test/%252e%252e/", + "/test/%25252e%25252e/", + "/test/%25%32%65%25%32%65/", + ] { + let endpoint = hyper_client::HostEndpoint::new(host, port, path); + let request = hyper_client::build_request( + Method::GET, + &endpoint, + &HashMap::new(), + None, + key_keeper_shared_state + .get_current_key_guid() + .await + .unwrap_or(None), + key_keeper_shared_state + .get_current_key_value() + .await + .unwrap_or(None), + ) + .unwrap(); + let response = hyper_client::send_request(host, port, request, logger::write_warning) + .await + .unwrap(); + assert_eq!( + http::StatusCode::NOT_FOUND, + response.status(), + "nested percent-encoded path {path:?} must return NOT_FOUND" + ); + } + // test large request body let body = vec![88u8; super::REQUEST_BODY_LOW_LIMIT_SIZE + 1]; let request = hyper_client::build_request(