fix(cli,core): keep secret-holding commands out of core dumps

Only `pdfs daemon` marked itself non-dumpable, though `mount`, `login`,
`unlock` and `logout` hold the same passphrases or tokens. All five now
do. The file-manager script also passes each path after `--`, so a file
named like an option stays a path (B240).
This commit is contained in:
nvrl committed 2026-10-16 11:58:28 +02:00
1 parent 4836c5a84a
commit 6456f503f1
3 files changed
+73 -4

No files matched your search

+26 -3
View File
@@ -831,6 +831,20 @@ fn main() -> Result<()> {
let cli = Cli::parse();
JSON_OUTPUT.store(cli.json, std::sync::atomic::Ordering::Relaxed);
// The commands that hold the password, the key passphrases or the session
// tokens in memory: keep them out of core dumps and away from same-user
// ptrace. Only the daemon did, though `mount` holds the same keys for as
// long as it runs.
if matches!(
cli.command,
Command::Login { .. }
| Command::Unlock { .. }
| Command::Logout
| Command::Mount { .. }
| Command::Daemon { .. }
) {
pdfs_core::kernelkey::forbid_dumps();
}
match cli.command {
Command::Login { username, remember } => cmd_login(username, remember),
Command::Unlock { remember } => cmd_unlock(remember),
@@ -2227,9 +2241,6 @@ where
/// an external unmount triggers a remount; errors back off and retry. This is
/// the entry point for the systemd user service.
fn cmd_daemon(mountpoint: Option<PathBuf>) -> Result<()> {
// The daemon holds the account's key passphrases in memory; keep them out
// of core dumps and away from same-user ptrace.
pdfs_core::kernelkey::forbid_dumps();
loop {
// Wait until a session is stored. The GUI enables this service on login,
// but the service may also start at boot before the user has logged in.
@@ -3787,6 +3798,18 @@ mod login_wait_tests {
assert_eq!(login_poll_delay(u32::MAX), Duration::from_secs(15));
}
/// The file-manager script passes each selected path after `--`, so a
/// file named like an option is still the path.
#[test]
fn a_path_after_a_double_dash_is_a_path_even_when_it_looks_like_a_flag() {
let cli = Cli::try_parse_from(["pdfs", "pin", "--", "--json"]).unwrap();
assert!(!cli.json);
match cli.command {
Command::Pin { path } => assert_eq!(path, PathBuf::from("--json")),
_ => panic!("not a pin"),
}
}
#[test]
fn a_waited_for_run_that_did_not_fully_succeed_exits_with_an_error() {
assert!(finished_run("the import", Some((0, false, None))).is_ok());
+16 -1
View File
@@ -23,6 +23,11 @@ const SCRIPTS: [(&str, &str); 2] = [
/// `NAUTILUS_SCRIPT_SELECTED_FILE_PATHS`; the positional args carry the same
/// selection when the script runs on a local (non-URI) folder, so we prefer the
/// environment and fall back to `"$@"` for the Nemo/Caja variants.
///
/// The `--` keeps a file named like an option, such as `--json`, a path. A
/// name with a newline in it still arrives split in two from Nautilus, whose
/// list is newline-separated; `pdfs` then fails on both halves, which touches
/// nothing.
fn script_body(subcommand: &str) -> String {
format!(
"#!/bin/sh\n\
@@ -41,7 +46,7 @@ fn script_body(subcommand: &str) -> String {
\n\
printf '%s' \"$paths\" | while IFS= read -r path; do\n\
\x20 [ -n \"$path\" ] || continue\n\
\x20 pdfs {subcommand} \"$path\" || exit 1\n\
\x20 pdfs {subcommand} -- \"$path\" || exit 1\n\
done\n"
)
}
@@ -95,3 +100,13 @@ fn write_scripts(dir: &PathBuf) -> io::Result<()> {
tracing::debug!("installed file-manager scripts in {dir:?}");
Ok(())
}
#[cfg(test)]
mod tests {
use super::script_body;
#[test]
fn the_script_ends_options_before_each_path() {
assert!(script_body("pin").contains(r#"pdfs pin -- "$path""#));
}
}
+31
View File
@@ -100,6 +100,7 @@ a live account since.
| Entry | Title |
|---|---|
| [B240](#b240--pdfs-mount-can-be-core-dumped-with-the-keys-in-memory) | pdfs mount can be core-dumped with the keys in memory |
| [B239](#b239----wait-reports-success-for-imports-and-re-dates-that-failed) | --wait reports success for imports and re-dates that failed |
| [B238](#b238--draining-a-large-offline-batch-is-quadratic-in-its-size) | Draining a large offline batch is quadratic in its size |
| [B237](#b237--every-reconnect-resends-refused-uploads-all-in-the-same-second) | Every reconnect resends refused uploads, all in the same second |
@@ -342,6 +343,36 @@ a live account since.
Newest first.
### B240 — pdfs mount can be core-dumped with the keys in memory
**Status:** Fixed, not verified live
**Found:** 2026-10-16, architecture audit (`FINDINGS.md` section 3, core dumps and the
file-manager script).
**Where:** `main` in `crates/pdfs-cli/src/main.rs`; `script_body` in
`crates/pdfs-core/src/shell.rs`
**Repro:**
- Run `pdfs mount` and send it `SIGSEGV`, with core dumps enabled. The dump holds the key
passphrases.
- In the mount, name a file `--json` and choose **Scripts → Keep offline** on it in Nautilus.
**Cause.** Only `pdfs daemon` marked itself non-dumpable. `pdfs mount`, `login`, `unlock` and
`logout` hold the password, the key passphrases or the session tokens too, and could be
core-dumped or ptraced by another process of the same user. Separately, the file-manager script
ran `pdfs pin "$path"`, so a file named like an option was read as one.
**Fix.** Those five commands mark themselves non-dumpable at start. The script passes each path
after `--`. A name with a newline still arrives split from Nautilus, whose list is
newline-separated; `pdfs` then fails on both halves and changes nothing.
The app's own login was left as it is: a non-dumpable GUI process hides its `/proc` entries from
the desktop portal, which identifies the caller through them.
**Test:** `tests::a_path_after_a_double_dash_is_a_path_even_when_it_looks_like_a_flag` in
`crates/pdfs-cli/src/main.rs`; `shell::tests::the_script_ends_options_before_each_path`.
---
### B239 — --wait reports success for imports and re-dates that failed
**Status:** Fixed, not verified live