From 6d5b183a3c5dc75cd6280feedd3c6809506d15ee Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 4 Jan 2025 16:16:54 +0000 Subject: [PATCH] Sanitize relative paths server-side --- backend/Cargo.lock | 10 ++++++++++ backend/sync_server/Cargo.toml | 1 + backend/sync_server/src/server/create_document.rs | 15 ++++++++++----- backend/sync_server/src/server/delete_document.rs | 6 +++--- backend/sync_server/src/server/update_document.rs | 12 ++++++++---- backend/sync_server/src/utils.rs | 12 ++++++++++++ 6 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 backend/sync_server/src/utils.rs diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 885470b7..7bb3bab2 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1610,6 +1610,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "sanitize-filename" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc984f4f9ceb736a7bb755c3e3bd17dc56370af2600c9780dcc48c66453da34d" +dependencies = [ + "regex", +] + [[package]] name = "schemars" version = "0.8.21" @@ -2121,6 +2130,7 @@ dependencies = [ "log", "rand", "reconcile", + "sanitize-filename", "schemars", "serde", "serde_yaml", diff --git a/backend/sync_server/Cargo.toml b/backend/sync_server/Cargo.toml index b79f8f65..bfb60f94 100644 --- a/backend/sync_server/Cargo.toml +++ b/backend/sync_server/Cargo.toml @@ -25,6 +25,7 @@ aide = { version = "0.13.4", features = ["axum", "axum-ws", "scalar", "axum-head schemars = { version = "0.8.21", features = ["chrono", "uuid1"] } tracing = "0.1" rand = "0.8.5" +sanitize-filename = "0.6.0" [lints] workspace = true diff --git a/backend/sync_server/src/server/create_document.rs b/backend/sync_server/src/server/create_document.rs index 900095cc..ac186d0c 100644 --- a/backend/sync_server/src/server/create_document.rs +++ b/backend/sync_server/src/server/create_document.rs @@ -12,11 +12,14 @@ use schemars::JsonSchema; use serde::Deserialize; use sync_lib::{base64_to_bytes, merge}; -use super::{auth::auth, requests::CreateDocumentVersion, responses::DocumentUpdateResponse}; +use super::{ + app_state::AppState, auth::auth, requests::CreateDocumentVersion, + responses::DocumentUpdateResponse, +}; use crate::{ - app_state::AppState, database::models::{StoredDocumentVersion, VaultId}, errors::{client_error, server_error, SyncServerError}, + utils::sanitize_path, }; // This is required for aide to infer the path parameter types and names @@ -49,9 +52,11 @@ pub async fn create_document( .await .map_err(server_error)?; + let sanitized_relative_path = sanitize_path(&request.relative_path); + let maybe_existing_version = state .database - .get_latest_document_by_path(&vault_id, &request.relative_path, Some(&mut transaction)) + .get_latest_document_by_path(&vault_id, &sanitized_relative_path, Some(&mut transaction)) .await .map_err(server_error)? .and_then(|doc| if doc.is_deleted { None } else { Some(doc) }); @@ -87,7 +92,7 @@ pub async fn create_document( let new_version = StoredDocumentVersion { vault_id, vault_update_id: last_update_id + 1, - relative_path: request.relative_path, + relative_path: sanitized_relative_path, document_id: existing_version.document_id, content: merged_content, created_date: request.created_date, @@ -107,7 +112,7 @@ pub async fn create_document( vault_id, vault_update_id: last_update_id + 1, document_id: uuid::Uuid::new_v4(), - relative_path: request.relative_path, + relative_path: sanitized_relative_path, content: content_bytes, created_date: request.created_date, updated_date: chrono::Utc::now(), diff --git a/backend/sync_server/src/server/delete_document.rs b/backend/sync_server/src/server/delete_document.rs index fd56ada9..83fcda83 100644 --- a/backend/sync_server/src/server/delete_document.rs +++ b/backend/sync_server/src/server/delete_document.rs @@ -10,11 +10,11 @@ use axum_extra::{ use schemars::JsonSchema; use serde::Deserialize; -use super::{auth::auth, requests::DeleteDocumentVersion}; +use super::{app_state::AppState, auth::auth, requests::DeleteDocumentVersion}; use crate::{ - app_state::AppState, database::models::{DocumentId, StoredDocumentVersion, VaultId}, errors::{server_error, SyncServerError}, + utils::sanitize_path, }; // This is required for aide to infer the path parameter types and names @@ -52,7 +52,7 @@ pub async fn delete_document( vault_id, vault_update_id: last_update_id + 1, document_id, - relative_path: request.relative_path, + relative_path: sanitize_path(&request.relative_path), content: vec![], created_date: request.created_date, updated_date: chrono::Utc::now(), diff --git a/backend/sync_server/src/server/update_document.rs b/backend/sync_server/src/server/update_document.rs index 50285399..31ab4320 100644 --- a/backend/sync_server/src/server/update_document.rs +++ b/backend/sync_server/src/server/update_document.rs @@ -12,11 +12,14 @@ use schemars::JsonSchema; use serde::Deserialize; use sync_lib::{base64_to_bytes, merge}; -use super::{auth::auth, requests::UpdateDocumentVersion, responses::DocumentUpdateResponse}; +use super::{ + app_state::AppState, auth::auth, requests::UpdateDocumentVersion, + responses::DocumentUpdateResponse, +}; use crate::{ - app_state::AppState, database::models::{DocumentId, StoredDocumentVersion, VaultId}, errors::{client_error, not_found_error, server_error, SyncServerError}, + utils::sanitize_path, }; // This is required for aide to infer the path parameter types and names @@ -84,10 +87,11 @@ pub async fn update_document( .context("Failed to decode base64 content in request") .map_err(client_error)?; + let sanitized_relative_path = sanitize_path(&request.relative_path); // Return the latest version if the content and path are the same as the latest // version if content_bytes == latest_version.content - && request.relative_path == latest_version.relative_path + && sanitized_relative_path == latest_version.relative_path { info!("Document content is the same as the latest version, skipping update"); transaction @@ -110,7 +114,7 @@ pub async fn update_document( // We can only update the relative path if we're the first one to do so let new_relative_path = if parent_document.relative_path == latest_version.relative_path { - request.relative_path.clone() + sanitized_relative_path } else { latest_version.relative_path.clone() }; diff --git a/backend/sync_server/src/utils.rs b/backend/sync_server/src/utils.rs new file mode 100644 index 00000000..dad45349 --- /dev/null +++ b/backend/sync_server/src/utils.rs @@ -0,0 +1,12 @@ +/// Sanitize the document's path to allow all clients to create the same path in +/// their filesystem. If we didn't do this server-side, client's would need to +/// deal with mapping invalid names to valid ones and then back. +pub fn sanitize_path(path: &str) -> String { + let options = sanitize_filename::Options { + truncate: true, + windows: true, // Windows is the lowest common denominator + replacement: "", + }; + + sanitize_filename::sanitize_with_options(path, options) +}