From 63577041ed8c7ae65ff444b1bf1869dc5350567d Mon Sep 17 00:00:00 2001 From: Kuba <78603704+jakub-tldr@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:39:28 +0200 Subject: [PATCH] reject non uuids instance ids --- src-tauri/Cargo.lock | 1 + src-tauri/Cargo.toml | 3 +- src-tauri/daemon/src/daemon.rs | 15 +++- .../enterprise/service-locations/Cargo.toml | 1 + .../enterprise/service-locations/src/lib.rs | 76 ++++++++++++++++++- .../enterprise/service-locations/src/linux.rs | 13 ++-- .../service-locations/src/windows.rs | 7 +- 7 files changed, 101 insertions(+), 15 deletions(-) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 664d4560..e70c7732 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1759,6 +1759,7 @@ dependencies = [ "tempfile", "thiserror 2.0.20", "tokio", + "uuid", "windows 0.62.2", "windows-acl", "windows-service", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index cc25a2a9..1abfd744 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -40,6 +40,7 @@ qrcode = { version = "0.14", features = ["image"] } tokio-tungstenite = { version = "0.30", features = ["native-tls"] } tokio-util = "0.7" url = "2" +uuid = { version = "1", features = ["v4"] } vergen-git2 = { version = "10.0", features = ["build"] } wiremock = "0.6" x25519-dalek = { version = "3.0", features = [ @@ -134,7 +135,7 @@ time = { version = "0.3", features = ["formatting", "macros"] } tokio.workspace = true tokio-util = "0.7" tonic.workspace = true -uuid = { version = "1", features = ["v4"] } +uuid.workspace = true tonic-prost.workspace = true tower = "0.5" tracing.workspace = true diff --git a/src-tauri/daemon/src/daemon.rs b/src-tauri/daemon/src/daemon.rs index 60b272d8..c586e740 100644 --- a/src-tauri/daemon/src/daemon.rs +++ b/src-tauri/daemon/src/daemon.rs @@ -25,7 +25,7 @@ use defguard_client_proto::defguard::{ use defguard_client_service_locations::reconciler::{run_reconciler, ReconcileSignal}; use defguard_client_service_locations::ServiceLocationError; #[cfg(any(windows, target_os = "linux"))] -use defguard_client_service_locations::ServiceLocationManager; +use defguard_client_service_locations::{validate_instance_id, ServiceLocationManager}; #[cfg(not(target_os = "macos"))] use defguard_wireguard_rs::Kernel; #[cfg(target_os = "macos")] @@ -222,7 +222,13 @@ impl DesktopDaemonService for DaemonService { request: tonic::Request, ) -> Result, Status> { debug!("Received a request to save service locations"); - let service_location = request.into_inner(); + let mut service_location = request.into_inner(); + service_location.instance_id = validate_instance_id(&service_location.instance_id) + .map_err(|err| { + let msg = format!("Failed to save service locations: {err}"); + error!("{msg}"); + Status::invalid_argument(msg) + })?; self.service_location_manager .write() @@ -259,6 +265,11 @@ impl DesktopDaemonService for DaemonService { ) -> Result, Status> { debug!("Received a request to delete service locations"); let instance_id = request.into_inner().instance_id; + let instance_id = validate_instance_id(&instance_id).map_err(|err| { + let msg = format!("Failed to delete service locations: {err}"); + error!("{msg}"); + Status::invalid_argument(msg) + })?; let mut manager = self.service_location_manager.write().unwrap(); manager diff --git a/src-tauri/enterprise/service-locations/Cargo.toml b/src-tauri/enterprise/service-locations/Cargo.toml index aca39aeb..c70f03fc 100644 --- a/src-tauri/enterprise/service-locations/Cargo.toml +++ b/src-tauri/enterprise/service-locations/Cargo.toml @@ -22,6 +22,7 @@ serde.workspace = true serde_json.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["time"] } +uuid.workspace = true [dev-dependencies] tempfile.workspace = true diff --git a/src-tauri/enterprise/service-locations/src/lib.rs b/src-tauri/enterprise/service-locations/src/lib.rs index 93e08915..2c028c9b 100644 --- a/src-tauri/enterprise/service-locations/src/lib.rs +++ b/src-tauri/enterprise/service-locations/src/lib.rs @@ -1,6 +1,6 @@ -#[cfg(any(windows, target_os = "linux", test))] -use std::ffi::OsStr; use std::{collections::HashMap, fmt, fs, path::Path, time::SystemTime}; +#[cfg(any(windows, target_os = "linux", test))] +use std::{ffi::OsStr, path::PathBuf}; use defguard_client_core::{ database::models::{ @@ -17,6 +17,7 @@ use defguard_wireguard_rs::{error::WireguardInterfaceError, WGApi}; use log::debug; use log::warn; use serde::{Deserialize, Serialize}; +use uuid::fmt::Hyphenated; #[cfg(target_os = "linux")] pub mod linux; @@ -34,6 +35,8 @@ pub enum ServiceLocationError { InitError(String), #[error("Failed to load service location storage: {0}")] LoadError(String), + #[error("Invalid instance ID: {0}")] + InvalidInstanceId(String), #[error(transparent)] Io(#[from] std::io::Error), #[error(transparent)] @@ -191,6 +194,21 @@ impl fmt::Debug for SingleServiceLocationData { } } +pub fn validate_instance_id(instance_id: &str) -> Result { + instance_id + .parse::() + .map(|uuid| uuid.to_string()) + .map_err(|_| ServiceLocationError::InvalidInstanceId(instance_id.to_string())) +} + +#[cfg(any(windows, target_os = "linux", test))] +fn instance_file_path( + directory: &Path, + instance_id: &str, +) -> Result { + Ok(directory.join(format!("{}.json", validate_instance_id(instance_id)?))) +} + /// Whether the file at `path` already holds exactly `contents`. Makes a save idempotent thus /// allowing pushing service locations on every poll cycle. /// @@ -284,6 +302,60 @@ pub fn to_service_location(location: &Location) -> Result PathBuf { PathBuf::from(DEFGUARD_DIR).join(SERVICE_LOCATIONS_SUBDIR) } -fn get_instance_file_path(instance_id: &str) -> PathBuf { - get_shared_directory().join(format!("{instance_id}.json")) +fn get_instance_file_path(instance_id: &str) -> Result { + instance_file_path(&get_shared_directory(), instance_id) } fn ensure_shared_directory() -> Result { @@ -119,7 +120,7 @@ impl ServiceLocationManager { ServiceLocationData::from_save_request(request, service_locations.clone()); ensure_shared_directory()?; - let instance_file_path = get_instance_file_path(instance_id); + let instance_file_path = get_instance_file_path(instance_id)?; let json = serde_json::to_string_pretty(&service_location_data)?; // Saving is pushed unconditionally on every poll cycle, so nothing having changed is the @@ -653,7 +654,7 @@ impl ServiceLocationManager { ) -> Result<(), ServiceLocationError> { debug!("Deleting Linux service locations for instance {instance_id}"); - let instance_file_path = get_instance_file_path(instance_id); + let instance_file_path = get_instance_file_path(instance_id)?; if instance_file_path.exists() { fs::remove_file(&instance_file_path)?; debug!("Deleted Linux service locations for instance {instance_id}"); @@ -676,7 +677,7 @@ impl ServiceLocationManager { &self, instance_id: &str, ) -> Result, ServiceLocationError> { - let instance_file_path = get_instance_file_path(instance_id); + let instance_file_path = get_instance_file_path(instance_id)?; load_service_locations_from_file(&instance_file_path) } } diff --git a/src-tauri/enterprise/service-locations/src/windows.rs b/src-tauri/enterprise/service-locations/src/windows.rs index 57b2f2bd..21fab206 100644 --- a/src-tauri/enterprise/service-locations/src/windows.rs +++ b/src-tauri/enterprise/service-locations/src/windows.rs @@ -28,7 +28,8 @@ use windows_acl::acl::ACL; use windows_sys::Win32::NetworkManagement::IpHelper::NotifyAddrChange; use crate::{ - is_unchanged_on_disk, load_service_locations_from_directory, load_service_locations_from_file, + instance_file_path, is_unchanged_on_disk, load_service_locations_from_directory, + load_service_locations_from_file, reconciler::{ reconcile_action, PostureAuthorizationRequest, PostureAuthorizations, ReconcileAction, ReconcileSignal, @@ -259,9 +260,7 @@ fn set_protected_acls(path: &str) -> Result<(), ServiceLocationError> { } fn get_instance_file_path(instance_id: &str) -> Result { - let mut path = get_shared_directory()?; - path.push(format!("{instance_id}.json")); - Ok(path) + instance_file_path(&get_shared_directory()?, instance_id) } pub(crate) fn is_user_logged_in() -> bool {