From 8013c6235de47adbf8cbfa3fe81f531f1e47193d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 28 Oct 2025 11:46:15 +0000 Subject: [PATCH 1/3] Feat: Sanitize header values to prevent injection attacks Co-authored-by: contact --- src/render.rs | 37 ++++++++++++++++--- .../it_works_newline_in_redirect_link.sql | 5 +++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/sql_test_files/it_works_newline_in_redirect_link.sql diff --git a/src/render.rs b/src/render.rs index c9229ce7..8f6fa8f5 100644 --- a/src/render.rs +++ b/src/render.rs @@ -167,7 +167,9 @@ impl HeaderContext { self.response.status(StatusCode::FOUND); self.has_status = true; } - self.response.insert_header((name.as_str(), value_str)); + let sanitized_value = sanitize_header_value(value_str); + self.response + .insert_header((name.as_str(), sanitized_value)); } Ok(self) } @@ -242,7 +244,9 @@ impl HeaderContext { self.has_status = true; let link = get_object_str(data, "link") .with_context(|| "The redirect component requires a 'link' property")?; - self.response.insert_header((header::LOCATION, link)); + let sanitized_link = sanitize_header_value(link); + self.response + .insert_header((header::LOCATION, sanitized_link)); let response = self.response.body(()); Ok(response) } @@ -288,9 +292,10 @@ impl HeaderContext { get_object_str(options, "filename").or_else(|| get_object_str(options, "title")) { let extension = if filename.contains('.') { "" } else { ".csv" }; + let sanitized_filename = sanitize_header_value(filename); self.response.insert_header(( header::CONTENT_DISPOSITION, - format!("attachment; filename={filename}{extension}"), + format!("attachment; filename={sanitized_filename}{extension}"), )); } let csv_renderer = CsvBodyRenderer::new(self.writer, options).await?; @@ -315,9 +320,10 @@ impl HeaderContext { log::debug!("Authentication failed"); // The authentication failed let http_response: HttpResponse = if let Some(link) = get_object_str(&data, "link") { + let sanitized_link = sanitize_header_value(link); self.response .status(StatusCode::FOUND) - .insert_header((header::LOCATION, link)) + .insert_header((header::LOCATION, sanitized_link)) .body( "Sorry, but you are not authorized to access this page. \ Redirecting to the login page...", @@ -333,9 +339,10 @@ impl HeaderContext { fn download(mut self, options: &JsonValue) -> anyhow::Result { if let Some(filename) = get_object_str(options, "filename") { + let sanitized_filename = sanitize_header_value(filename); self.response.insert_header(( header::CONTENT_DISPOSITION, - format!("attachment; filename=\"{filename}\""), + format!("attachment; filename=\"{sanitized_filename}\""), )); } let data_url = get_object_str(options, "data_url") @@ -407,6 +414,26 @@ async fn verify_password_async( .await? } +fn sanitize_header_value(value: &str) -> String { + let sanitized: String = value + .chars() + .filter(|&c| { + let byte = c as u32; + byte >= 0x20 && byte != 0x7F + }) + .collect(); + + if sanitized != value { + log::warn!( + "Sanitized header value by removing control characters. Original length: {}, Sanitized length: {}", + value.len(), + sanitized.len() + ); + } + + sanitized +} + fn get_object_str<'a>(json: &'a JsonValue, key: &str) -> Option<&'a str> { json.as_object() .and_then(|obj| obj.get(key)) diff --git a/tests/sql_test_files/it_works_newline_in_redirect_link.sql b/tests/sql_test_files/it_works_newline_in_redirect_link.sql new file mode 100644 index 00000000..ca1f8b46 --- /dev/null +++ b/tests/sql_test_files/it_works_newline_in_redirect_link.sql @@ -0,0 +1,5 @@ +select 'redirect' as component, 'line1 +line2 +line3 +' as link; +select 'text' as component, 'It works !' AS contents; From 8afa988d11e924c27078f609a8b7806311df68e7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 28 Oct 2025 13:16:28 +0000 Subject: [PATCH 2/3] Refactor sanitize_header_value to use Cow and remove test file Co-authored-by: contact --- src/render.rs | 39 ++++++++++--------- .../it_works_newline_in_redirect_link.sql | 5 --- 2 files changed, 20 insertions(+), 24 deletions(-) delete mode 100644 tests/sql_test_files/it_works_newline_in_redirect_link.sql diff --git a/src/render.rs b/src/render.rs index 8f6fa8f5..d76e5129 100644 --- a/src/render.rs +++ b/src/render.rs @@ -169,7 +169,7 @@ impl HeaderContext { } let sanitized_value = sanitize_header_value(value_str); self.response - .insert_header((name.as_str(), sanitized_value)); + .insert_header((name.as_str(), sanitized_value.as_ref())); } Ok(self) } @@ -246,7 +246,7 @@ impl HeaderContext { .with_context(|| "The redirect component requires a 'link' property")?; let sanitized_link = sanitize_header_value(link); self.response - .insert_header((header::LOCATION, sanitized_link)); + .insert_header((header::LOCATION, sanitized_link.as_ref())); let response = self.response.body(()); Ok(response) } @@ -323,7 +323,7 @@ impl HeaderContext { let sanitized_link = sanitize_header_value(link); self.response .status(StatusCode::FOUND) - .insert_header((header::LOCATION, sanitized_link)) + .insert_header((header::LOCATION, sanitized_link.as_ref())) .body( "Sorry, but you are not authorized to access this page. \ Redirecting to the login page...", @@ -414,24 +414,25 @@ async fn verify_password_async( .await? } -fn sanitize_header_value(value: &str) -> String { - let sanitized: String = value - .chars() - .filter(|&c| { - let byte = c as u32; - byte >= 0x20 && byte != 0x7F - }) - .collect(); - - if sanitized != value { - log::warn!( - "Sanitized header value by removing control characters. Original length: {}, Sanitized length: {}", - value.len(), - sanitized.len() - ); +fn sanitize_header_value(value: &str) -> Cow<'_, str> { + if value.bytes().all(|b| b >= 0x20 && b != 0x7F) { + return Cow::Borrowed(value); } - sanitized + log::warn!( + "Sanitized header value by removing control characters. Original length: {}", + value.len() + ); + + Cow::Owned( + value + .chars() + .filter(|&c| { + let byte = c as u32; + byte >= 0x20 && byte != 0x7F + }) + .collect(), + ) } fn get_object_str<'a>(json: &'a JsonValue, key: &str) -> Option<&'a str> { diff --git a/tests/sql_test_files/it_works_newline_in_redirect_link.sql b/tests/sql_test_files/it_works_newline_in_redirect_link.sql deleted file mode 100644 index ca1f8b46..00000000 --- a/tests/sql_test_files/it_works_newline_in_redirect_link.sql +++ /dev/null @@ -1,5 +0,0 @@ -select 'redirect' as component, 'line1 -line2 -line3 -' as link; -select 'text' as component, 'It works !' AS contents; From 105fbd319447f9214e77560d8aa6e871323953f0 Mon Sep 17 00:00:00 2001 From: lovasoa Date: Tue, 28 Oct 2025 18:17:07 +0100 Subject: [PATCH 3/3] refactor header error handling in render.rs --- CHANGELOG.md | 5 ++++ src/render.rs | 76 ++++++++++++++++++++------------------------------- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbe9532f..1dce2696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG.md +## v0.39.1 (unreleased) + - More precise server timing tracking to debug performance issues + - Fix missing server timing header in some cases + - Implement nice error messages for some header-related errors such as invalid header values. + ## v0.39.0 (2025-10-28) - Ability to execute sql for URL paths with another extension. If you create sitemap.xml.sql, it will be executed for example.com/sitemap.xml - Display source line info in errors even when the database does not return a precise error position. In this case, the entire problematic SQL statement is referenced. diff --git a/src/render.rs b/src/render.rs index 499ac558..337d08f5 100644 --- a/src/render.rs +++ b/src/render.rs @@ -46,8 +46,10 @@ use crate::webserver::http::RequestContext; use crate::webserver::response_writer::{AsyncResponseWriter, ResponseWriter}; use crate::webserver::ErrorWithStatus; use crate::AppState; +use actix_web::body::MessageBody; use actix_web::cookie::time::format_description::well_known::Rfc3339; use actix_web::cookie::time::OffsetDateTime; +use actix_web::http::header::TryIntoHeaderPair; use actix_web::http::{header, StatusCode}; use actix_web::{HttpResponse, HttpResponseBuilder}; use anyhow::{bail, format_err, Context as AnyhowContext}; @@ -116,7 +118,7 @@ impl HeaderContext { Some(HeaderComponent::HttpHeader) => { self.add_http_header(&data).map(PageContext::Header) } - Some(HeaderComponent::Redirect) => self.redirect(&data).map(PageContext::Close), + Some(HeaderComponent::Redirect) => self.redirect(&data), Some(HeaderComponent::Json) => self.json(&data), Some(HeaderComponent::Csv) => self.csv(&data).await, Some(HeaderComponent::Cookie) => self.add_cookie(&data).map(PageContext::Header), @@ -167,9 +169,9 @@ impl HeaderContext { self.response.status(StatusCode::FOUND); self.has_status = true; } - let sanitized_value = sanitize_header_value(value_str); - self.response - .insert_header((name.as_str(), sanitized_value.as_ref())); + let header = TryIntoHeaderPair::try_into_pair((name.as_str(), value_str)) + .map_err(|e| anyhow::anyhow!("Invalid header: {name}:{value_str}: {e:#?}"))?; + self.response.insert_header(header); } Ok(self) } @@ -239,15 +241,13 @@ impl HeaderContext { Ok(self) } - fn redirect(mut self, data: &JsonValue) -> anyhow::Result { + fn redirect(mut self, data: &JsonValue) -> anyhow::Result { self.response.status(StatusCode::FOUND); self.has_status = true; let link = get_object_str(data, "link") .with_context(|| "The redirect component requires a 'link' property")?; - let sanitized_link = sanitize_header_value(link); - self.response - .insert_header((header::LOCATION, sanitized_link.as_ref())); - Ok(self.into_response_builder().body(())) + self.response.insert_header((header::LOCATION, link)); + self.close_with_body(()) } /// Answers to the HTTP request with a single json object @@ -260,9 +260,7 @@ impl HeaderContext { } else { serde_json::to_vec(contents)? }; - Ok(PageContext::Close( - self.into_response_builder().body(json_response), - )) + self.close_with_body(json_response) } else { let body_type = get_object_str(data, "type"); let json_renderer = match body_type { @@ -293,10 +291,9 @@ impl HeaderContext { get_object_str(options, "filename").or_else(|| get_object_str(options, "title")) { let extension = if filename.contains('.') { "" } else { ".csv" }; - let sanitized_filename = sanitize_header_value(filename); self.response.insert_header(( header::CONTENT_DISPOSITION, - format!("attachment; filename={sanitized_filename}{extension}"), + format!("attachment; filename={filename}{extension}"), )); } let csv_renderer = CsvBodyRenderer::new(self.writer, options).await?; @@ -321,15 +318,15 @@ impl HeaderContext { log::debug!("Authentication failed"); // The authentication failed if let Some(link) = get_object_str(&data, "link") { - let sanitized_link = sanitize_header_value(link); self.response .status(StatusCode::FOUND) - .insert_header((header::LOCATION, sanitized_link.as_ref())); + .insert_header((header::LOCATION, link)); self.has_status = true; - Ok(PageContext::Close(self.into_response_builder().body( + let response = self.into_response( "Sorry, but you are not authorized to access this page. \ Redirecting to the login page...", - ))) + )?; + Ok(PageContext::Close(response)) } else { anyhow::bail!(ErrorWithStatus { status: StatusCode::UNAUTHORIZED @@ -339,10 +336,9 @@ impl HeaderContext { fn download(mut self, options: &JsonValue) -> anyhow::Result { if let Some(filename) = get_object_str(options, "filename") { - let sanitized_filename = sanitize_header_value(filename); self.response.insert_header(( header::CONTENT_DISPOSITION, - format!("attachment; filename=\"{sanitized_filename}\""), + format!("attachment; filename=\"{filename}\""), )); } let data_url = get_object_str(options, "data_url") @@ -365,9 +361,7 @@ impl HeaderContext { self.response .insert_header((header::CONTENT_TYPE, content_type)); } - Ok(PageContext::Close( - self.into_response_builder().body(body_bytes.into_owned()), - )) + self.close_with_body(body_bytes.into_owned()) } fn log(self, data: &JsonValue) -> anyhow::Result { @@ -381,9 +375,18 @@ impl HeaderContext { } } - fn into_response_builder(mut self) -> HttpResponseBuilder { + fn into_response(mut self, body: B) -> anyhow::Result { self.add_server_timing_header(); - self.response + match self.response.message_body(body) { + Ok(response) => Ok(response.map_into_boxed_body()), + Err(e) => Err(anyhow::anyhow!( + "An error occured while generating the request headers: {e:#}" + )), + } + } + + fn close_with_body(self, body: B) -> anyhow::Result { + Ok(PageContext::Close(self.into_response(body)?)) } async fn start_body(mut self, data: JsonValue) -> anyhow::Result { @@ -401,7 +404,7 @@ impl HeaderContext { } pub fn close(self) -> HttpResponse { - self.into_response_builder().finish() + self.into_response(()).unwrap() } } @@ -418,27 +421,6 @@ async fn verify_password_async( .await? } -fn sanitize_header_value(value: &str) -> Cow<'_, str> { - if value.bytes().all(|b| b >= 0x20 && b != 0x7F) { - return Cow::Borrowed(value); - } - - log::warn!( - "Sanitized header value by removing control characters. Original length: {}", - value.len() - ); - - Cow::Owned( - value - .chars() - .filter(|&c| { - let byte = c as u32; - byte >= 0x20 && byte != 0x7F - }) - .collect(), - ) -} - fn get_object_str<'a>(json: &'a JsonValue, key: &str) -> Option<&'a str> { json.as_object() .and_then(|obj| obj.get(key))