Fixes pentest issue DG25-10 from 2025-09-02

crates/defguard_core/Cargo.toml

@@ -84,6 +84,7 @@ strum_macros = { workspace = true }
 bytes = { workspace = true }
 ed25519-dalek = { version = "2.2", features = ["rand_core"] }
 tower = "0.5"
+regex = "1.10"
 
 [dev-dependencies]
 bytes = "1.6"

crates/defguard_core/src/db/models/mod.rs

@@ -143,7 +143,7 @@ impl UserInfo {
     ///
     /// Return `true` if groups were changed, `false` otherwise.
     pub(crate) async fn handle_user_groups(
-        &mut self,
+        &self,
         transaction: &mut PgConnection,
         user: &mut User<Id>,
     ) -> Result<GroupDiff, SqlxError> {

crates/defguard_core/src/grpc/enrollment.rs

@@ -48,6 +48,7 @@ use crate::{
         user::check_password_strength,
     },
     headers::get_device_info,
+    is_valid_phone_number,
     mail::Mail,
     server_config,
     templates::{self, TemplateLocation},
@@ -343,6 +344,19 @@ impl EnrollmentServer {
         Ok(())
     }
 
+    fn validate_activated_user(&self, request: &ActivateUserRequest) -> Result<(), Status> {
+        if let Some(ref phone_number) = request.phone_number {
+            if !is_valid_phone_number(phone_number) {
+                return Err(Status::new(
+                    tonic::Code::InvalidArgument,
+                    "invalid phone number",
+                ));
+            }
+        }
+
+        Ok(())
+    }
+
     #[instrument(skip_all)]
     pub async fn activate_user(
         &self,
@@ -351,6 +365,7 @@ impl EnrollmentServer {
     ) -> Result<(), Status> {
         debug!("Activating user account: {request:?}");
         let enrollment = self.validate_session(request.token.as_ref()).await?;
+        self.validate_activated_user(&request)?;
 
         let ip_address;
         let device_info;

crates/defguard_core/src/handlers/user.rs

@@ -31,6 +31,7 @@ use crate::{
     },
     error::WebError,
     events::{ApiEvent, ApiEventType, ApiRequestContext},
+    is_valid_phone_number,
     mail::Mail,
     server_config, templates,
 };
@@ -280,6 +281,7 @@ pub async fn add_user(
             status: StatusCode::BAD_REQUEST,
         });
     }
+
     // check if email doesn't already exist
     if User::find_by_email(&appstate.pool, &user_data.email)
         .await?
@@ -291,6 +293,18 @@ pub async fn add_user(
             status: StatusCode::BAD_REQUEST,
         });
     }
+
+    // check phone number
+    if let Some(ref phone) = user_data.phone {
+        if !is_valid_phone_number(phone) {
+            debug!("Invalid phone number for new user {username}: {phone}");
+            return Ok(ApiResponse {
+                json: json!({}),
+                status: StatusCode::BAD_REQUEST,
+            });
+        }
+    }
+
     let password = match &user_data.password {
         Some(password) => {
             // check password strength
@@ -645,11 +659,12 @@ pub async fn modify_user(
     context: ApiRequestContext,
     State(appstate): State<AppState>,
     Path(username): Path<String>,
-    Json(mut user_info): Json<UserInfo>,
+    Json(user_info): Json<UserInfo>,
 ) -> ApiResult {
     debug!("User {} updating user {username}", session.user.username);
     let mut user = user_for_admin_or_self(&appstate.pool, &session, &username).await?;
     let groups_before = UserInfo::from_user(&appstate.pool, &user).await?.groups;
+
     // store user before mods
     let before = user.clone();
     let old_username = user.username.clone();
@@ -660,6 +675,18 @@ pub async fn modify_user(
             status: StatusCode::BAD_REQUEST,
         });
     }
+
+    // check phone number
+    if let Some(ref phone) = user_info.phone {
+        if !is_valid_phone_number(phone) {
+            debug!("Invalid phone number for user {username}: {phone}");
+            return Ok(ApiResponse {
+                json: json!({}),
+                status: StatusCode::BAD_REQUEST,
+            });
+        }
+    }
+
     let status_changing = user_info.is_active != user.is_active;
 
     let mut transaction = appstate.pool.begin().await?;

crates/defguard_core/src/lib.rs

@@ -3,7 +3,7 @@
 #![allow(clippy::result_large_err)]
 use std::{
     net::{IpAddr, Ipv4Addr, SocketAddr},
-    sync::{Arc, Mutex, OnceLock, RwLock},
+    sync::{Arc, LazyLock, Mutex, OnceLock, RwLock},
 };
 
 use crate::version::IncompatibleComponents;
@@ -60,6 +60,7 @@ use handlers::{
     yubikey::{delete_yubikey, rename_yubikey},
 };
 use ipnetwork::IpNetwork;
+use regex::Regex;
 use secrecy::ExposeSecret;
 use semver::Version;
 use sqlx::PgPool;
@@ -193,6 +194,11 @@ pub(crate) fn server_config() -> &'static DefGuardConfig {
         .expect("Server configuration not set yet")
 }
 
+static PHONE_NUMBER_REGEX: LazyLock<Regex> = LazyLock::new(|| {
+    Regex::new(r"^(\+?\d{1,3}\s?)?(\(\d{1,3}\)|\d{1,3})[-\s]?\d{1,4}[-\s]?\d{1,4}?$")
+        .expect("Failed to parse phone number regex")
+});
+
 // WireGuard key length in bytes.
 pub(crate) const KEY_LENGTH: usize = 32;
 
@@ -964,3 +970,40 @@ where
             .join(",")
     }
 }
+
+pub(crate) fn is_valid_phone_number(number: &str) -> bool {
+    PHONE_NUMBER_REGEX.is_match(number)
+}
+
+#[cfg(test)]
+mod test {
+
+    use super::is_valid_phone_number;
+
+    #[test]
+    fn test_is_valid_phone_number_dg25_10() {
+        let valid_numbers = &[
+            "+48 (91) 123-456",
+            "123 456 7890",
+            "+1 (202) 555-0173",
+            "91-1234-5678",
+            "(22) 567 890",
+        ];
+        for number in valid_numbers {
+            assert!(is_valid_phone_number(number));
+        }
+
+        let invalid_numbers = &[
+            "4*4",
+            "+48  123456789",
+            "123-456-789-0000",
+            "(+48) 123 456",
+            "202.555.0173",
+            "(12345) 6789",
+            "+48 (91) 123-456 000 111",
+        ];
+        for number in invalid_numbers {
+            assert!(!is_valid_phone_number(number));
+        }
+    }
+}