diff --git a/Cargo.lock b/Cargo.lock index 0500f9e..4227497 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -687,7 +687,7 @@ dependencies = [ [[package]] name = "moongreet" -version = "0.3.0" +version = "0.3.1" dependencies = [ "env_logger", "gdk-pixbuf", diff --git a/config/moongreet.toml b/config/moongreet.toml index 01557e0..7bfd4fb 100644 --- a/config/moongreet.toml +++ b/config/moongreet.toml @@ -5,4 +5,4 @@ # Absolute path to wallpaper image background = "/usr/share/backgrounds/wallpaper.jpg" # GTK theme for the greeter UI -gtk-theme = "catppuccin-mocha-lavender-standard+default" +gtk-theme = "Colloid-Catppuccin" diff --git a/src/greeter.rs b/src/greeter.rs index 384f050..6e5b889 100644 --- a/src/greeter.rs +++ b/src/greeter.rs @@ -147,8 +147,16 @@ pub fn create_greeter_window( // Apply GTK theme from config if let Some(ref theme_name) = config.gtk_theme { - if let Some(settings) = gtk::Settings::default() { - settings.set_gtk_theme_name(Some(theme_name)); + if !theme_name.is_empty() + && theme_name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '-' | '+' | '.')) + { + if let Some(settings) = gtk::Settings::default() { + settings.set_gtk_theme_name(Some(theme_name)); + } + } else { + log::warn!("Ignoring invalid GTK theme name: {theme_name}"); } } @@ -606,7 +614,9 @@ fn set_default_avatar( if let Ok(loader) = gdk_pixbuf::PixbufLoader::with_type("svg") { loader.set_size(AVATAR_SIZE, AVATAR_SIZE); if loader.write(svg_bytes).is_ok() { - let _ = loader.close(); + if let Err(e) = loader.close() { + log::warn!("Failed to close SVG loader: {e}"); + } if let Some(pixbuf) = loader.pixbuf() { let texture = gdk::Texture::for_pixbuf(&pixbuf); state.borrow_mut().default_avatar_texture = Some(texture.clone()); @@ -629,8 +639,11 @@ fn get_selected_session( if sessions.is_empty() { return None; } - let idx = dropdown.selected() as usize; - sessions.get(idx).cloned() + let idx = dropdown.selected(); + if idx == gtk::INVALID_LIST_POSITION { + return None; + } + sessions.get(idx as usize).cloned() } /// Pre-select the last used session for a user in the dropdown. @@ -795,6 +808,7 @@ fn attempt_login( &sock_path, &greetd_sock, &login_cancelled, + strings, ) }) .await; @@ -869,14 +883,19 @@ fn login_worker( sock_path: &str, greetd_sock: &Arc>>, login_cancelled: &Arc, + strings: &Strings, ) -> Result { if login_cancelled.load(std::sync::atomic::Ordering::SeqCst) { return Ok(LoginResult::Cancelled); } let mut sock = UnixStream::connect(sock_path).map_err(|e| e.to_string())?; - sock.set_read_timeout(Some(std::time::Duration::from_secs(10))) - .ok(); + if let Err(e) = sock.set_read_timeout(Some(std::time::Duration::from_secs(10))) { + log::warn!("Failed to set read timeout: {e}"); + } + if let Err(e) = sock.set_write_timeout(Some(std::time::Duration::from_secs(10))) { + log::warn!("Failed to set write timeout: {e}"); + } { let mut guard = greetd_sock.lock().map_err(|e| e.to_string())?; *guard = Some(sock.try_clone().map_err(|e| e.to_string())?); @@ -900,7 +919,7 @@ fn login_worker( message: response .get("description") .and_then(|v| v.as_str()) - .unwrap_or("Authentication failed") + .unwrap_or(strings.auth_failed) .to_string(), }); } @@ -927,7 +946,7 @@ fn login_worker( // Multi-stage auth is not supported let _ = ipc::cancel_session(&mut sock); return Ok(LoginResult::Error { - message: "Multi-stage authentication is not supported".to_string(), + message: strings.multi_stage_unsupported.to_string(), }); } } @@ -939,17 +958,18 @@ fn login_worker( _ => { let _ = ipc::cancel_session(&mut sock); return Ok(LoginResult::Error { - message: "Invalid session command".to_string(), + message: strings.invalid_session_command.to_string(), }); } }; - // Validate: first token must be an absolute path to an existing file - let binary = std::path::Path::new(&cmd[0]); - if !binary.is_absolute() || !binary.is_file() { + // Validate: reject obviously invalid commands (empty, null bytes, path traversal) + // greetd resolves PATH for relative commands like "niri-session" + let first = &cmd[0]; + if first.is_empty() || first.contains('\0') || first.contains("..") { let _ = ipc::cancel_session(&mut sock); return Ok(LoginResult::Error { - message: "Invalid session command".to_string(), + message: strings.invalid_session_command.to_string(), }); } @@ -969,14 +989,14 @@ fn login_worker( message: response .get("description") .and_then(|v| v.as_str()) - .unwrap_or("Failed to start session") + .unwrap_or(strings.session_start_failed) .to_string(), }); } } Ok(LoginResult::Error { - message: "Unexpected response from greetd".to_string(), + message: strings.unexpected_greetd_response.to_string(), }) } @@ -1012,7 +1032,11 @@ fn execute_power_action( // -- Last user/session persistence -- fn load_last_user() -> Option { - let content = std::fs::read_to_string(LAST_USER_PATH).ok()?; + load_last_user_from(Path::new(LAST_USER_PATH)) +} + +fn load_last_user_from(path: &Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; let username = content.trim(); if is_valid_username(username) { Some(username.to_string()) @@ -1022,15 +1046,29 @@ fn load_last_user() -> Option { } fn save_last_user(username: &str) { - let path = Path::new(LAST_USER_PATH); + save_last_user_to(Path::new(LAST_USER_PATH), username); +} + +fn save_last_user_to(path: &Path, username: &str) { if let Some(parent) = path.parent() { let _ = std::fs::create_dir_all(parent); } - let _ = std::fs::write(path, username); + use std::os::unix::fs::OpenOptionsExt; + use std::io::Write; + let _ = std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(path) + .and_then(|mut f| f.write_all(username.as_bytes())); } fn load_last_session(username: &str) -> Option { - let path = Path::new(LAST_SESSION_DIR).join(username); + load_last_session_from(&Path::new(LAST_SESSION_DIR).join(username)) +} + +fn load_last_session_from(path: &Path) -> Option { let content = std::fs::read_to_string(path).ok()?; let name = content.trim(); if is_valid_session_name(name) { @@ -1059,7 +1097,19 @@ fn save_last_session(username: &str, session_name: &str) { } let dir = Path::new(LAST_SESSION_DIR); let _ = std::fs::create_dir_all(dir); - let _ = std::fs::write(dir.join(username), session_name); + save_last_session_to(&dir.join(username), session_name); +} + +fn save_last_session_to(path: &Path, session_name: &str) { + use std::os::unix::fs::OpenOptionsExt; + use std::io::Write; + let _ = std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(path) + .and_then(|mut f| f.write_all(session_name.as_bytes())); } #[cfg(test)] @@ -1103,26 +1153,71 @@ mod tests { } #[test] - fn last_user_persistence() { + fn last_user_roundtrip() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("last-user"); - std::fs::write(&path, "alice").unwrap(); - - let content = std::fs::read_to_string(&path).unwrap(); - let username = content.trim(); - assert!(is_valid_username(username)); - assert_eq!(username, "alice"); + save_last_user_to(&path, "alice"); + let loaded = load_last_user_from(&path); + assert_eq!(loaded, Some("alice".to_string())); } #[test] - fn last_session_persistence() { + fn last_user_rejects_invalid() { let dir = tempfile::tempdir().unwrap(); - let session_dir = dir.path().join("last-session"); - std::fs::create_dir_all(&session_dir).unwrap(); - std::fs::write(session_dir.join("alice"), "Niri").unwrap(); + let path = dir.path().join("last-user"); + save_last_user_to(&path, "../evil"); + let loaded = load_last_user_from(&path); + assert_eq!(loaded, None); + } - let content = std::fs::read_to_string(session_dir.join("alice")).unwrap(); - assert_eq!(content.trim(), "Niri"); + #[test] + fn last_user_missing_file() { + let loaded = load_last_user_from(Path::new("/nonexistent/last-user")); + assert_eq!(loaded, None); + } + + #[test] + fn last_session_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("alice"); + save_last_session_to(&path, "Niri"); + let loaded = load_last_session_from(&path); + assert_eq!(loaded, Some("Niri".to_string())); + } + + #[test] + fn last_session_rejects_invalid() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("alice"); + save_last_session_to(&path, "../../../etc/evil"); + let loaded = load_last_session_from(&path); + assert_eq!(loaded, None); + } + + #[test] + fn last_session_missing_file() { + let loaded = load_last_session_from(Path::new("/nonexistent/session")); + assert_eq!(loaded, None); + } + + #[test] + fn last_user_file_permissions() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("last-user"); + save_last_user_to(&path, "alice"); + let meta = std::fs::metadata(&path).unwrap(); + use std::os::unix::fs::PermissionsExt; + assert_eq!(meta.permissions().mode() & 0o777, 0o600); + } + + #[test] + fn last_session_file_permissions() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("session"); + save_last_session_to(&path, "Niri"); + let meta = std::fs::metadata(&path).unwrap(); + use std::os::unix::fs::PermissionsExt; + assert_eq!(meta.permissions().mode() & 0o777, 0o600); } // -- split_shell_words tests -- @@ -1259,6 +1354,7 @@ mod tests { let result = login_worker( "alice", "wrongpass", "/usr/bin/niri", &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), ); let result = result.unwrap(); @@ -1300,6 +1396,7 @@ mod tests { let result = login_worker( "alice", "correct", "/usr/bin/bash", &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), ); let result = result.unwrap(); @@ -1334,6 +1431,7 @@ mod tests { let result = login_worker( "alice", "pass", "/usr/bin/niri", &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), ); let result = result.unwrap(); @@ -1370,6 +1468,7 @@ mod tests { let result = login_worker( "alice", "pass", "/usr/bin/bash", &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), ); let result = result.unwrap(); @@ -1384,6 +1483,7 @@ mod tests { let result = login_worker( "alice", "pass", "/usr/bin/niri", "/nonexistent/sock", &default_greetd_sock(), &cancelled, + load_strings(Some("en")), ); let result = result.unwrap(); @@ -1405,19 +1505,52 @@ mod tests { let _msg = ipc::recv_message(stream).unwrap(); ipc::send_message(stream, &serde_json::json!({"type": "success"})).unwrap(); - // cancel_session (from invalid exec_cmd path) + // cancel_session (from invalid exec_cmd with path traversal) let _msg = ipc::recv_message(stream).unwrap(); ipc::send_message(stream, &serde_json::json!({"type": "success"})).unwrap(); }); - // Non-absolute exec_cmd + // exec_cmd with path traversal let result = login_worker( - "alice", "pass", "relative-binary", + "alice", "pass", "../../../etc/evil", &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), ); let result = result.unwrap(); assert!(matches!(result, LoginResult::Error { .. })); handle.join().unwrap(); } + + #[test] + fn login_worker_relative_exec_cmd_allowed() { + let (sock_path, handle) = fake_greetd(|stream| { + // create_session + let _msg = ipc::recv_message(stream).unwrap(); + ipc::send_message(stream, &serde_json::json!({ + "type": "auth_message", + "auth_message_type": "secret", + "auth_message": "Password: ", + })).unwrap(); + + // post_auth_response → success + let _msg = ipc::recv_message(stream).unwrap(); + ipc::send_message(stream, &serde_json::json!({"type": "success"})).unwrap(); + + // start_session with relative command (e.g. niri-session) + let _msg = ipc::recv_message(stream).unwrap(); + ipc::send_message(stream, &serde_json::json!({"type": "success"})).unwrap(); + }); + + // Relative exec_cmd like "niri-session" should be allowed + let result = login_worker( + "alice", "pass", "niri-session", + &sock_path, &default_greetd_sock(), &default_cancelled(), + load_strings(Some("en")), + ); + + let result = result.unwrap(); + assert!(matches!(result, LoginResult::Success { .. })); + handle.join().unwrap(); + } } diff --git a/src/i18n.rs b/src/i18n.rs index 73a0c9f..c15bfe5 100644 --- a/src/i18n.rs +++ b/src/i18n.rs @@ -30,6 +30,7 @@ pub struct Strings { pub shutdown_failed: &'static str, pub connection_error: &'static str, pub socket_error: &'static str, + pub unexpected_greetd_response: &'static str, // Templates (use .replace("{n}", &count.to_string())) pub faillock_attempts_remaining: &'static str, @@ -54,6 +55,7 @@ const STRINGS_DE: Strings = Strings { shutdown_failed: "Herunterfahren fehlgeschlagen", connection_error: "Verbindungsfehler", socket_error: "Socket-Fehler", + unexpected_greetd_response: "Unerwartete Antwort von greetd", faillock_attempts_remaining: "Noch {n} Versuch(e) vor Kontosperrung!", faillock_locked: "Konto ist möglicherweise gesperrt", }; @@ -76,6 +78,7 @@ const STRINGS_EN: Strings = Strings { shutdown_failed: "Shutdown failed", connection_error: "Connection error", socket_error: "Socket error", + unexpected_greetd_response: "Unexpected response from greetd", faillock_attempts_remaining: "{n} attempt(s) remaining before lockout!", faillock_locked: "Account may be locked", }; @@ -281,6 +284,7 @@ mod tests { assert!(!s.shutdown_failed.is_empty(), "{locale}: shutdown_failed"); assert!(!s.faillock_attempts_remaining.is_empty(), "{locale}: faillock_attempts_remaining"); assert!(!s.faillock_locked.is_empty(), "{locale}: faillock_locked"); + assert!(!s.unexpected_greetd_response.is_empty(), "{locale}: unexpected_greetd_response"); } } diff --git a/src/ipc.rs b/src/ipc.rs index 2377859..51d9f7a 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -75,7 +75,6 @@ fn recv_payload(stream: &mut UnixStream, n: usize) -> Result, IpcError> } /// Send a length-prefixed JSON message to the greetd socket. -/// Header and payload are sent in a single write for atomicity. pub fn send_message( stream: &mut UnixStream, msg: &serde_json::Value, @@ -86,10 +85,8 @@ pub fn send_message( } let header = (payload.len() as u32).to_le_bytes(); - let mut buf = Vec::with_capacity(4 + payload.len()); - buf.extend_from_slice(&header); - buf.extend_from_slice(&payload); - stream.write_all(&buf)?; + stream.write_all(&header)?; + stream.write_all(&payload)?; Ok(()) } diff --git a/src/main.rs b/src/main.rs index 7f68d9f..e650ca7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,9 +88,11 @@ fn setup_logging() { let log_dir = PathBuf::from("/var/cache/moongreet"); if log_dir.is_dir() { let log_file = log_dir.join("moongreet.log"); + use std::os::unix::fs::OpenOptionsExt; if let Ok(file) = std::fs::OpenOptions::new() .create(true) .append(true) + .mode(0o640) .open(&log_file) { builder.target(env_logger::Target::Pipe(Box::new(file))); diff --git a/src/power.rs b/src/power.rs index b9f3ca1..9e3d0a3 100644 --- a/src/power.rs +++ b/src/power.rs @@ -7,7 +7,6 @@ use std::process::Command; #[derive(Debug)] pub enum PowerError { CommandFailed { action: &'static str, message: String }, - Timeout { action: &'static str }, } impl fmt::Display for PowerError { @@ -16,9 +15,6 @@ impl fmt::Display for PowerError { PowerError::CommandFailed { action, message } => { write!(f, "{action} failed: {message}") } - PowerError::Timeout { action } => { - write!(f, "{action} timed out") - } } } } @@ -76,12 +72,6 @@ mod tests { assert_eq!(err.to_string(), "reboot failed: No such file or directory"); } - #[test] - fn power_error_timeout_display() { - let err = PowerError::Timeout { action: "shutdown" }; - assert_eq!(err.to_string(), "shutdown timed out"); - } - #[test] fn run_command_returns_error_for_missing_binary() { let result = run_command("test", "nonexistent-binary-xyz", &[]); diff --git a/src/users.rs b/src/users.rs index 2f9e6c7..981efa9 100644 --- a/src/users.rs +++ b/src/users.rs @@ -106,14 +106,18 @@ pub fn get_avatar_path_with( // AccountsService icon takes priority if accountsservice_dir.exists() { let icon = accountsservice_dir.join(username); - if icon.exists() && !icon.is_symlink() { + if let Ok(meta) = icon.symlink_metadata() + && !meta.file_type().is_symlink() + { return Some(icon); } } // ~/.face fallback let face = home.join(".face"); - if face.exists() && !face.is_symlink() { + if let Ok(meta) = face.symlink_metadata() + && !meta.file_type().is_symlink() + { return Some(face); }