fix: audit findings — wallpaper safety, log filtering, error truncation (v0.4.1)
- 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
This commit is contained in:
parent
4c9b436978
commit
14d6476e5a
@ -59,6 +59,9 @@ cd pkg && makepkg -sf && sudo pacman -U moongreet-git-<version>-x86_64.pkg.tar.z
|
|||||||
- **Symmetrie mit moonset**: Gleiche Patterns (i18n, config, users, power, GResource)
|
- **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
|
- **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
|
- **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
|
- **File Permissions**: Cache-Dateien 0o600
|
||||||
- **Testbare Persistence**: `save_*_to`/`load_*_from` Varianten mit konfigurierbarem Pfad für Unit-Tests
|
- **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
|
||||||
|
|||||||
2
Cargo.lock
generated
2
Cargo.lock
generated
@ -569,7 +569,7 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "moongreet"
|
name = "moongreet"
|
||||||
version = "0.4.0"
|
version = "0.4.1"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"gdk-pixbuf",
|
"gdk-pixbuf",
|
||||||
"gdk4",
|
"gdk4",
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "moongreet"
|
name = "moongreet"
|
||||||
version = "0.4.0"
|
version = "0.4.1"
|
||||||
edition = "2024"
|
edition = "2024"
|
||||||
description = "A greetd greeter for Wayland with GTK4 and Layer Shell"
|
description = "A greetd greeter for Wayland with GTK4 and Layer Shell"
|
||||||
license = "MIT"
|
license = "MIT"
|
||||||
|
|||||||
7
DECISIONS.md
Normal file
7
DECISIONS.md
Normal file
@ -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()`
|
||||||
@ -4,7 +4,7 @@
|
|||||||
# Maintainer: Dominik Kressler
|
# Maintainer: Dominik Kressler
|
||||||
|
|
||||||
pkgname=moongreet-git
|
pkgname=moongreet-git
|
||||||
pkgver=0.3.0.r0.g0000000
|
pkgver=0.3.1.r5.g4c9b436
|
||||||
pkgrel=1
|
pkgrel=1
|
||||||
pkgdesc="A greetd greeter for Wayland with GTK4 and Layer Shell"
|
pkgdesc="A greetd greeter for Wayland with GTK4 and Layer Shell"
|
||||||
arch=('x86_64')
|
arch=('x86_64')
|
||||||
|
|||||||
108
src/greeter.rs
108
src/greeter.rs
@ -22,6 +22,7 @@ use crate::users::{self, User};
|
|||||||
|
|
||||||
const AVATAR_SIZE: i32 = 128;
|
const AVATAR_SIZE: i32 = 128;
|
||||||
const MAX_AVATAR_FILE_SIZE: u64 = 10 * 1024 * 1024;
|
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_USER_PATH: &str = "/var/cache/moongreet/last-user";
|
||||||
const LAST_SESSION_DIR: &str = "/var/cache/moongreet/last-session";
|
const LAST_SESSION_DIR: &str = "/var/cache/moongreet/last-session";
|
||||||
const MAX_USERNAME_LENGTH: usize = 256;
|
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 == '-')
|
.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<gdk::Texture> {
|
||||||
|
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.
|
/// Create a wallpaper-only window for secondary monitors.
|
||||||
pub fn create_wallpaper_window(
|
pub fn create_wallpaper_window(
|
||||||
bg_path: &Path,
|
texture: &gdk::Texture,
|
||||||
app: >k::Application,
|
app: >k::Application,
|
||||||
) -> gtk::ApplicationWindow {
|
) -> gtk::ApplicationWindow {
|
||||||
let window = gtk::ApplicationWindow::builder()
|
let window = gtk::ApplicationWindow::builder()
|
||||||
@ -104,19 +145,15 @@ pub fn create_wallpaper_window(
|
|||||||
.build();
|
.build();
|
||||||
window.add_css_class("wallpaper");
|
window.add_css_class("wallpaper");
|
||||||
|
|
||||||
let background = create_background_picture(bg_path);
|
let background = create_background_picture(texture);
|
||||||
window.set_child(Some(&background));
|
window.set_child(Some(&background));
|
||||||
|
|
||||||
window
|
window
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Create a Picture widget for the wallpaper background.
|
/// Create a Picture widget for the wallpaper background from a pre-loaded texture.
|
||||||
fn create_background_picture(bg_path: &Path) -> gtk::Picture {
|
fn create_background_picture(texture: &gdk::Texture) -> gtk::Picture {
|
||||||
let background = if bg_path.starts_with("/dev/moonarch/moongreet") {
|
let background = gtk::Picture::for_paintable(texture);
|
||||||
gtk::Picture::for_resource(bg_path.to_str().unwrap_or(""))
|
|
||||||
} else {
|
|
||||||
gtk::Picture::for_filename(bg_path.to_str().unwrap_or(""))
|
|
||||||
};
|
|
||||||
background.set_content_fit(gtk::ContentFit::Cover);
|
background.set_content_fit(gtk::ContentFit::Cover);
|
||||||
background.set_hexpand(true);
|
background.set_hexpand(true);
|
||||||
background.set_vexpand(true);
|
background.set_vexpand(true);
|
||||||
@ -135,7 +172,7 @@ struct GreeterState {
|
|||||||
|
|
||||||
/// Create the main greeter window with login UI.
|
/// Create the main greeter window with login UI.
|
||||||
pub fn create_greeter_window(
|
pub fn create_greeter_window(
|
||||||
bg_path: &Path,
|
texture: Option<&gdk::Texture>,
|
||||||
config: &Config,
|
config: &Config,
|
||||||
app: >k::Application,
|
app: >k::Application,
|
||||||
) -> gtk::ApplicationWindow {
|
) -> gtk::ApplicationWindow {
|
||||||
@ -182,7 +219,9 @@ pub fn create_greeter_window(
|
|||||||
window.set_child(Some(&overlay));
|
window.set_child(Some(&overlay));
|
||||||
|
|
||||||
// Background wallpaper
|
// 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)
|
// Main layout: 3 rows (top spacer, center login, bottom bar)
|
||||||
let main_box = gtk::Box::new(gtk::Orientation::Vertical, 0);
|
let main_box = gtk::Box::new(gtk::Orientation::Vertical, 0);
|
||||||
@ -934,13 +973,16 @@ fn login_worker(
|
|||||||
return Ok(LoginResult::Cancelled);
|
return Ok(LoginResult::Cancelled);
|
||||||
}
|
}
|
||||||
if response.get("type").and_then(|v| v.as_str()) == Some("error") {
|
if response.get("type").and_then(|v| v.as_str()) == Some("error") {
|
||||||
return Ok(LoginResult::Error {
|
let description = response
|
||||||
message: response
|
.get("description")
|
||||||
.get("description")
|
.and_then(|v| v.as_str())
|
||||||
.and_then(|v| v.as_str())
|
.unwrap_or("");
|
||||||
.unwrap_or(strings.auth_failed)
|
let message = if !description.is_empty() && description.len() <= MAX_GREETD_ERROR_LENGTH {
|
||||||
.to_string(),
|
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 { .. }));
|
assert!(matches!(result, LoginResult::Success { .. }));
|
||||||
handle.join().unwrap();
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
21
src/main.rs
21
src/main.rs
@ -54,18 +54,26 @@ fn activate(app: >k::Application) {
|
|||||||
let bg_path = config::resolve_background_path(&config);
|
let bg_path = config::resolve_background_path(&config);
|
||||||
log::debug!("Background path: {}", bg_path.display());
|
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();
|
let use_layer_shell = std::env::var("MOONGREET_NO_LAYER_SHELL").is_err();
|
||||||
log::debug!("Layer shell: {use_layer_shell}");
|
log::debug!("Layer shell: {use_layer_shell}");
|
||||||
|
|
||||||
// Main greeter window (login UI) — compositor picks focused monitor
|
// 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 {
|
if use_layer_shell {
|
||||||
setup_layer_shell(&greeter_window, true, gtk4_layer_shell::Layer::Top);
|
setup_layer_shell(&greeter_window, true, gtk4_layer_shell::Layer::Top);
|
||||||
}
|
}
|
||||||
greeter_window.present();
|
greeter_window.present();
|
||||||
|
|
||||||
// Wallpaper-only windows on all monitors (only with layer shell)
|
// 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();
|
let monitors = display.monitors();
|
||||||
log::debug!("Monitor count: {}", monitors.n_items());
|
log::debug!("Monitor count: {}", monitors.n_items());
|
||||||
for i in 0..monitors.n_items() {
|
for i in 0..monitors.n_items() {
|
||||||
@ -73,7 +81,7 @@ fn activate(app: >k::Application) {
|
|||||||
.item(i)
|
.item(i)
|
||||||
.and_then(|obj| obj.downcast::<gdk::Monitor>().ok())
|
.and_then(|obj| obj.downcast::<gdk::Monitor>().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);
|
setup_layer_shell(&wallpaper, false, gtk4_layer_shell::Layer::Bottom);
|
||||||
wallpaper.set_monitor(Some(&monitor));
|
wallpaper.set_monitor(Some(&monitor));
|
||||||
wallpaper.present();
|
wallpaper.present();
|
||||||
@ -87,7 +95,12 @@ fn setup_logging() {
|
|||||||
.unwrap()
|
.unwrap()
|
||||||
.install()
|
.install()
|
||||||
.unwrap();
|
.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() {
|
fn main() {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user