fix: disable core dumps in release builds (v0.6.20)
moonlock dumped core on nearly every unlock — a GTK 4.22 regression in gtk4-layer-shell's session-lock teardown (upstream PR #125, not our code). The non-zeroizable password copies (GLib GString, PAM strdup) could land in those on-disk dumps. - main.rs: PR_SET_DUMPABLE(0) in release via disable_core_dumps(), set before any secret enters memory; also blocks non-root ptrace. Debug builds stay dumpable. - The crash itself is upstream and unfixed here; see DECISIONS.md.
This commit is contained in:
@@ -52,6 +52,7 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock
|
||||
- PAM callback: msg_style-aware (password only on PAM_PROMPT_ECHO_OFF), strdup-OOM-safe, num_msg guard against negative values
|
||||
- fprintd: the D-Bus signal sender is validated against fprintd's unique bus name (anti-spoofing)
|
||||
- Password: Zeroizing<String> from GTK entry extraction, Zeroizing<CString> in the PAM FFI layer (known limitation: the GLib GString and the strdup copy in PAM are not zeroized — an inherent GTK/libc limitation)
|
||||
- Core dumps: disabled in release builds via `PR_SET_DUMPABLE(0)` (`main.rs`, right after `setup_logging()`). The non-zeroizable password copies above can therefore never reach an on-disk core dump, and non-root `ptrace` of the running lockscreen is blocked. Debug builds stay dumpable so the dev-mode lockscreen remains debuggable.
|
||||
- Fingerprint unlock: calls unlock_callback directly after a verify match (no additional PAM acct check, because the PAM stack contains only `auth include login` — like swaylock); FP lockout is implemented via MAX_FP_ATTEMPTS in fingerprint.rs
|
||||
- PAM stack: only `auth include login` (standard pattern like swaylock/gtklock); no `account`/`session` stack, because the lockscreen does not manage a session lifecycle and `pam_unix(account)` requires setuid root
|
||||
- The wallpaper is loaded before lock() — connect_monitor fires during lock() and needs the texture; local JPEG loading is fast enough
|
||||
|
||||
Generated
+1
-1
@@ -575,7 +575,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "moonlock"
|
||||
version = "0.6.19"
|
||||
version = "0.6.20"
|
||||
dependencies = [
|
||||
"gdk-pixbuf",
|
||||
"gdk4",
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "moonlock"
|
||||
version = "0.6.19"
|
||||
version = "0.6.20"
|
||||
edition = "2024"
|
||||
description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support"
|
||||
license = "MIT"
|
||||
|
||||
@@ -2,6 +2,13 @@
|
||||
|
||||
Architectural and design decisions for Moonlock, in reverse chronological order.
|
||||
|
||||
## 2026-07-08 – Unlock SIGSEGV root-caused to a GTK 4.22 regression in gtk4-layer-shell (not our code); harden against core dumps
|
||||
|
||||
- **Who**: ClaudeCode, Dom
|
||||
- **Why**: `moonlock` was leaving a SIGSEGV core dump on essentially every unlock — 394 dumps since **2026-04-08 10:24**, and **zero** before that, i.e. 24 minutes after the system upgrade `gtk4 1:4.20.4 → 1:4.22.2` that morning. All backtraces are identical: `gtk_session_lock_instance_unlock` → `clear_lock_state` → `gtk_lock_surface_unmap_window` → `gtk_window_destroy` → `GtkApplication::window-removed` (a `VOID__OBJECT` marshal) → `gdk_surface_get_display: assertion 'GDK_IS_SURFACE (surface)' failed`. This is the **same symptom** as the v0.6.9–v0.6.17 crash hunt, which misdiagnosed it as our own `app.quit()` double-destroy and fixed a real-but-secondary double-destroy in v0.6.17 (quit in `::unlocked`, never destroy windows ourselves). The residual crash persisted because the true cause is upstream and was never correlated with the GTK 4.22 bump. Root cause: `gtk4-layer-shell` 1.3.0 `gtk_lock_surface_unmap_window()` unrealizes the lock window **before** destroying it; on GTK 4.22+, `gtk_window_destroy()` removes the window from its `GtkApplication`, and the `window-removed` handler dereferences the already-freed `GdkSurface`. Fixed upstream in [PR #125](https://github.com/wmww/gtk4-layer-shell/pull/125) (merge `cbfdd73`, 2026-06-15): destroy first, unrealize only if still realized. The fix is in `main` only — the latest release is 1.3.0 (Oct 2024), and Arch `extra` ships `1.3.0-1` without it. `moonset`/`moongreet` (plain layer-shell, no session-lock) never crash, confirming the fault is isolated to the session-lock teardown path.
|
||||
- **Tradeoffs**: (1) **Not security-relevant for access control** — `unlock()` sends the Wayland unlock (`session_lock_unlock`) *before* the crashing window teardown, so the crash is post-unlock cleanup on an already-authenticated path; and any lock-client death *without* a sent unlock keeps the session locked per ext-session-lock-v1 (enforced by the compositor/niri — the Rust panic hook does not apply, SIGSEGV bypasses it). Reframed from an initial over-cautious "session unlocks itself" read. (2) **Did not change our window handling.** Both upstream examples use app-associated windows (`session-lock.c`: `gtk_application_window_new(app)`; `session-lock.py`: `Gtk.Window(application=app)`) and upstream fixed the *library* while leaving the examples unchanged — so the pattern is endorsed and our `ApplicationWindow(application=app)` is correct. Reworking it to dodge the bug was rejected as working around an already-fixed upstream bug by deviating from the supported pattern. (3) **Did not override the system `gtk4-layer-shell`.** It is a shared library (waybar, moonset, moongreet all link it); shadowing it system-wide via `[moonarch]` to fix a moonlock-only unlock path is disproportionate and forks an established `extra` package (maintenance + stale-override risk). Delivery of the real fix is left to the upstream/Arch channel. (4) The core-dump hardening is **release-gated** (matching the file's existing release-only hardening — `exit(1)` without session-lock, `MOONLOCK_DEBUG` gating) so dev builds stay debuggable.
|
||||
- **How**: (1) `main.rs`: new `disable_core_dumps()` (`#[cfg(not(debug_assertions))]`) calling `libc::prctl(PR_SET_DUMPABLE, 0, …)`, invoked in `main()` right after `setup_logging()` and before any secret enters memory. Suppresses on-disk dumps of the non-zeroizable password copies (GLib GString, PAM strdup — documented limitation) and blocks non-root `ptrace`. `CLAUDE.md` Security section updated. Verified: `cargo build --release` compiles the release-gated path. (2) Deleted the 132 existing moonlock core dumps (~1.2 GB, `root:root 0640`) from `/var/lib/systemd/coredump/`. (3) **Open item, not done here**: deliver PR #125 to the system — report to the Arch `extra` maintainer for a `1.3.0-2` backport. Until then the crash persists (now without a dump); no fix for it lives in this repo, because the fix is not ours to make.
|
||||
|
||||
## 2026-06-17 – Harden avatar load against symlink TOCTOU via a shared O_NOFOLLOW loader (v0.6.19)
|
||||
|
||||
- **Who**: ClaudeCode, Dom
|
||||
|
||||
+24
@@ -275,6 +275,26 @@ fn setup_logging() {
|
||||
log::set_max_level(level);
|
||||
}
|
||||
|
||||
/// Disable core dumps for this process (release builds only).
|
||||
///
|
||||
/// moonlock holds the user's password in memory during authentication. Our own
|
||||
/// copies use `Zeroizing`, but the GLib GString from the entry and the strdup
|
||||
/// copy inside the PAM FFI layer are not zeroizable — on a crash they could be
|
||||
/// written to a core dump on disk. PR_SET_DUMPABLE(0) suppresses the dump and
|
||||
/// additionally blocks non-root ptrace of the running lockscreen.
|
||||
/// Debug builds stay dumpable so the dev-mode lockscreen remains debuggable.
|
||||
#[cfg(not(debug_assertions))]
|
||||
fn disable_core_dumps() {
|
||||
// SAFETY: prctl(PR_SET_DUMPABLE, 0) takes scalar args and has no memory effects.
|
||||
let rc = unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0, 0, 0, 0) };
|
||||
if rc != 0 {
|
||||
log::warn!(
|
||||
"Failed to disable core dumps (PR_SET_DUMPABLE): {}",
|
||||
std::io::Error::last_os_error()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn install_panic_hook() {
|
||||
// Install a panic hook BEFORE starting the app.
|
||||
// On panic, we log but NEVER unlock. The compositor's ext-session-lock-v1
|
||||
@@ -290,6 +310,10 @@ fn main() {
|
||||
install_panic_hook();
|
||||
setup_logging();
|
||||
|
||||
// Suppress core dumps before any secret enters memory (release only).
|
||||
#[cfg(not(debug_assertions))]
|
||||
disable_core_dumps();
|
||||
|
||||
// Root check — moonlock should not run as root
|
||||
if nix::unistd::getuid().is_root() {
|
||||
log::error!("Moonlock should not run as root");
|
||||
|
||||
Reference in New Issue
Block a user