From 14d6476e5a50d832337cba1b385d8ecaff04a915 Mon Sep 17 00:00:00 2001 From: nevaforget Date: Sat, 28 Mar 2026 10:29:21 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20findings=20=E2=80=94=20wallpaper?= =?UTF-8?q?=20safety,=20log=20filtering,=20error=20truncation=20(v0.4.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rework load_background_texture(): use resources_lookup_data()/from_bytes() for GResource path (no abort on missing resource), add 50 MB file size limit, handle non-UTF-8 paths gracefully - Filter error details to debug level only — warn! logs without internal details to prevent system info leaking into journal - Make debug logging opt-in via MOONGREET_DEBUG env var (default: Info) - Truncate greetd error description in stale-session retry path using MAX_GREETD_ERROR_LENGTH (matching show_greetd_error()) - Add 3 unit tests for load_background_texture edge cases --- CLAUDE.md | 5 ++- Cargo.lock | 2 +- Cargo.toml | 2 +- DECISIONS.md | 7 ++++ pkg/PKGBUILD | 2 +- src/greeter.rs | 108 ++++++++++++++++++++++++++++++++++++++++--------- src/main.rs | 21 ++++++++-- 7 files changed, 121 insertions(+), 26 deletions(-) create mode 100644 DECISIONS.md diff --git a/CLAUDE.md b/CLAUDE.md index f3c239a..e7f466f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,9 @@ cd pkg && makepkg -sf && sudo pacman -U moongreet-git--x86_64.pkg.tar.z - **Symmetrie mit moonset**: Gleiche Patterns (i18n, config, users, power, GResource) - **Session-Validierung**: Relative Pfade erlaubt (greetd löst PATH auf), nur `..`/Null-Bytes werden abgelehnt - **GTK-Theme-Validierung**: Nur alphanumerisch + `_-+.` erlaubt, verhindert Path-Traversal über Config -- **Journal-Logging**: `systemd-journal-logger` statt File-Logging — `journalctl -t moongreet` +- **Journal-Logging**: `systemd-journal-logger` statt File-Logging — `journalctl -t moongreet`, Debug-Level per `MOONGREET_DEBUG` Env-Var - **File Permissions**: Cache-Dateien 0o600 - **Testbare Persistence**: `save_*_to`/`load_*_from` Varianten mit konfigurierbarem Pfad für Unit-Tests +- **Shared Wallpaper Texture**: `gdk::Texture` wird einmal in `load_background_texture()` dekodiert und per Ref-Count an alle Fenster (Greeter + Wallpaper-Windows) geteilt — vermeidet redundante JPEG-Dekodierung pro Monitor +- **Wallpaper-Validierung**: GResource-Zweig via `resources_lookup_data()` + `from_bytes()` (kein Abort bei fehlendem Pfad), Dateigröße-Limit 50 MB, non-UTF-8-Pfade → `None` +- **Error-Detail-Filterung**: GDK/greetd-Fehlerdetails nur auf `debug!`-Level, `warn!` ohne interne Details — verhindert Systeminfo-Leak ins Journal diff --git a/Cargo.lock b/Cargo.lock index 8ee77ba..40e607f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -569,7 +569,7 @@ dependencies = [ [[package]] name = "moongreet" -version = "0.4.0" +version = "0.4.1" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index 4dfb7b2..4abdcf3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moongreet" -version = "0.4.0" +version = "0.4.1" edition = "2024" description = "A greetd greeter for Wayland with GTK4 and Layer Shell" license = "MIT" diff --git a/DECISIONS.md b/DECISIONS.md new file mode 100644 index 0000000..99c4ed4 --- /dev/null +++ b/DECISIONS.md @@ -0,0 +1,7 @@ +# Decisions + +## 2026-03-28 – Audit fixes for shared wallpaper texture (v0.4.1) +- **Who**: Selene, Dominik +- **Why**: Quality, performance, and security audits flagged issues in `load_background_texture()`, debug logging, and greetd error handling +- **Tradeoffs**: GResource path now requires UTF-8 (returns `None` for non-UTF-8 instead of aborting); 50 MB wallpaper limit is generous but prevents OOM; debug logging off by default trades observability for security +- **How**: GResource branch via `resources_lookup_data()` + `from_bytes()` (no abort), file size limit, error details only at debug level, `MOONGREET_DEBUG` env var for log level, greetd retry path truncation matching `show_greetd_error()` diff --git a/pkg/PKGBUILD b/pkg/PKGBUILD index 327c080..86fecdb 100644 --- a/pkg/PKGBUILD +++ b/pkg/PKGBUILD @@ -4,7 +4,7 @@ # Maintainer: Dominik Kressler pkgname=moongreet-git -pkgver=0.3.0.r0.g0000000 +pkgver=0.3.1.r5.g4c9b436 pkgrel=1 pkgdesc="A greetd greeter for Wayland with GTK4 and Layer Shell" arch=('x86_64') diff --git a/src/greeter.rs b/src/greeter.rs index 4bc9e51..38c419c 100644 --- a/src/greeter.rs +++ b/src/greeter.rs @@ -22,6 +22,7 @@ use crate::users::{self, User}; const AVATAR_SIZE: i32 = 128; const MAX_AVATAR_FILE_SIZE: u64 = 10 * 1024 * 1024; +const MAX_WALLPAPER_FILE_SIZE: u64 = 50 * 1024 * 1024; const LAST_USER_PATH: &str = "/var/cache/moongreet/last-user"; const LAST_SESSION_DIR: &str = "/var/cache/moongreet/last-session"; const MAX_USERNAME_LENGTH: usize = 256; @@ -94,9 +95,49 @@ fn is_valid_username(name: &str) -> bool { .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.' || c == '-') } +/// Load the background image as a shared texture (decode once, reuse everywhere). +pub fn load_background_texture(bg_path: &Path) -> Option { + let path_str = bg_path.to_str()?; + if bg_path.starts_with("/dev/moonarch/moongreet") { + match gio::resources_lookup_data(path_str, gio::ResourceLookupFlags::NONE) { + Ok(bytes) => match gdk::Texture::from_bytes(&bytes) { + Ok(texture) => Some(texture), + Err(e) => { + log::debug!("GResource texture decode error: {e}"); + log::warn!("Failed to decode background texture from GResource {path_str}"); + None + } + }, + Err(e) => { + log::debug!("GResource lookup error: {e}"); + log::warn!("Failed to load background texture from GResource {path_str}"); + None + } + } + } else { + if let Ok(meta) = std::fs::metadata(bg_path) + && meta.len() > MAX_WALLPAPER_FILE_SIZE + { + log::warn!( + "Wallpaper file too large ({} bytes), skipping: {}", + meta.len(), bg_path.display() + ); + return None; + } + match gdk::Texture::from_filename(bg_path) { + Ok(texture) => Some(texture), + Err(e) => { + log::debug!("Wallpaper load error: {e}"); + log::warn!("Failed to load background texture from {}", bg_path.display()); + None + } + } + } +} + /// Create a wallpaper-only window for secondary monitors. pub fn create_wallpaper_window( - bg_path: &Path, + texture: &gdk::Texture, app: >k::Application, ) -> gtk::ApplicationWindow { let window = gtk::ApplicationWindow::builder() @@ -104,19 +145,15 @@ pub fn create_wallpaper_window( .build(); window.add_css_class("wallpaper"); - let background = create_background_picture(bg_path); + let background = create_background_picture(texture); window.set_child(Some(&background)); window } -/// Create a Picture widget for the wallpaper background. -fn create_background_picture(bg_path: &Path) -> gtk::Picture { - let background = if bg_path.starts_with("/dev/moonarch/moongreet") { - gtk::Picture::for_resource(bg_path.to_str().unwrap_or("")) - } else { - gtk::Picture::for_filename(bg_path.to_str().unwrap_or("")) - }; +/// Create a Picture widget for the wallpaper background from a pre-loaded texture. +fn create_background_picture(texture: &gdk::Texture) -> gtk::Picture { + let background = gtk::Picture::for_paintable(texture); background.set_content_fit(gtk::ContentFit::Cover); background.set_hexpand(true); background.set_vexpand(true); @@ -135,7 +172,7 @@ struct GreeterState { /// Create the main greeter window with login UI. pub fn create_greeter_window( - bg_path: &Path, + texture: Option<&gdk::Texture>, config: &Config, app: >k::Application, ) -> gtk::ApplicationWindow { @@ -182,7 +219,9 @@ pub fn create_greeter_window( window.set_child(Some(&overlay)); // Background wallpaper - overlay.set_child(Some(&create_background_picture(bg_path))); + if let Some(texture) = texture { + overlay.set_child(Some(&create_background_picture(texture))); + } // Main layout: 3 rows (top spacer, center login, bottom bar) let main_box = gtk::Box::new(gtk::Orientation::Vertical, 0); @@ -934,13 +973,16 @@ fn login_worker( return Ok(LoginResult::Cancelled); } if response.get("type").and_then(|v| v.as_str()) == Some("error") { - return Ok(LoginResult::Error { - message: response - .get("description") - .and_then(|v| v.as_str()) - .unwrap_or(strings.auth_failed) - .to_string(), - }); + let description = response + .get("description") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let message = if !description.is_empty() && description.len() <= MAX_GREETD_ERROR_LENGTH { + description.to_string() + } else { + strings.auth_failed.to_string() + }; + return Ok(LoginResult::Error { message }); } } @@ -1581,4 +1623,34 @@ mod tests { assert!(matches!(result, LoginResult::Success { .. })); handle.join().unwrap(); } + + // -- load_background_texture tests -- + + #[test] + fn load_background_texture_missing_file_returns_none() { + let result = load_background_texture(Path::new("/nonexistent/wallpaper.jpg")); + assert!(result.is_none()); + } + + #[test] + fn load_background_texture_oversized_file_returns_none() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("huge.jpg"); + // Create a sparse file that exceeds MAX_WALLPAPER_FILE_SIZE + let f = std::fs::File::create(&path).unwrap(); + f.set_len(MAX_WALLPAPER_FILE_SIZE + 1).unwrap(); + let result = load_background_texture(&path); + assert!(result.is_none()); + } + + #[test] + fn load_background_texture_non_utf8_path_returns_none() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + // 0xFF is not valid UTF-8 + let non_utf8 = OsStr::from_bytes(&[0xff, 0xfe, 0xfd]); + let path = Path::new(non_utf8); + let result = load_background_texture(path); + assert!(result.is_none()); + } } diff --git a/src/main.rs b/src/main.rs index 2780081..f3a2717 100644 --- a/src/main.rs +++ b/src/main.rs @@ -54,18 +54,26 @@ fn activate(app: >k::Application) { let bg_path = config::resolve_background_path(&config); log::debug!("Background path: {}", bg_path.display()); + // Load background texture once — shared across all windows + let bg_texture = greeter::load_background_texture(&bg_path); + if bg_texture.is_none() { + log::error!("Failed to load background texture — greeter will start without wallpaper"); + } + let use_layer_shell = std::env::var("MOONGREET_NO_LAYER_SHELL").is_err(); log::debug!("Layer shell: {use_layer_shell}"); // Main greeter window (login UI) — compositor picks focused monitor - let greeter_window = greeter::create_greeter_window(&bg_path, &config, app); + let greeter_window = greeter::create_greeter_window(bg_texture.as_ref(), &config, app); if use_layer_shell { setup_layer_shell(&greeter_window, true, gtk4_layer_shell::Layer::Top); } greeter_window.present(); // Wallpaper-only windows on all monitors (only with layer shell) - if use_layer_shell { + if use_layer_shell + && let Some(ref texture) = bg_texture + { let monitors = display.monitors(); log::debug!("Monitor count: {}", monitors.n_items()); for i in 0..monitors.n_items() { @@ -73,7 +81,7 @@ fn activate(app: >k::Application) { .item(i) .and_then(|obj| obj.downcast::().ok()) { - let wallpaper = greeter::create_wallpaper_window(&bg_path, app); + let wallpaper = greeter::create_wallpaper_window(texture, app); setup_layer_shell(&wallpaper, false, gtk4_layer_shell::Layer::Bottom); wallpaper.set_monitor(Some(&monitor)); wallpaper.present(); @@ -87,7 +95,12 @@ fn setup_logging() { .unwrap() .install() .unwrap(); - log::set_max_level(log::LevelFilter::Debug); + let level = if std::env::var("MOONGREET_DEBUG").is_ok() { + log::LevelFilter::Debug + } else { + log::LevelFilter::Info + }; + log::set_max_level(level); } fn main() {