19 findings: 4 Critical, 4 High, 3 Medium, 4 Low, 4 Info. All Critical and High findings fixed.
C1 — Command injection / text IPC protocol
Original lock daemon protocol used "LOCK <url>\n". A URL containing \n injected extra commands into the child's command loop. An attacker who tricked the user into opening a crafted URL could unlock or overwrite any database the user had locked.
Fix: replaced entire protocol with length-prefixed binary frames. See 04-lockd.
C2 — World-writable cache directory
create_directories() inherits the process umask (often 0022 → 0755). Any local user could read the cached (encrypted) database and run offline attacks, or create symlinks to cause writes to go elsewhere.
Fix: explicit chmod(dir, 0700) after create_directories().
C3 — TOCTOU plugin load
Loader called access() to check the .so, then open() for mmap, then dlopen() on the path — three separate kernel operations on the same path. A race between any two allowed substituting a malicious library.
Fix: one open(O_RDONLY | O_NOFOLLOW) → mmap on that fd → dlopen via /proc/self/fd/<n>.
C4 — Cross-protocol curl redirect
CURLOPT_FOLLOWLOCATION was enabled with no protocol restriction. A malicious server could 302 a PUT to file:///etc/shadow, causing pwsafe to overwrite local files.
Fix: CURLOPT_PROTOCOLS_STR = "https,http", CURLOPT_REDIR_PROTOCOLS_STR = "https,http", FOLLOWLOCATION = 0.
H5 — Scheme allows '/' → path traversal in plugin filename
Without validation, a scheme like ../evil would produce plugin filename pwsafe-../evil.so — escaping the plugin directory. Fix: RFC 3986 §3.1 validation — scheme must match [a-zA-Z][a-zA-Z0-9+\-.]*.
H6 — Symlink attack via file plugin copy_file
The file plugin used fs::copy_file with no symlink check. An attacker could replace the destination with a symlink to overwrite arbitrary files. Partially mitigated by the C2 cache directory fix (0700 permissions make symlink creation much harder).
H7 — No explicit SSL VERIFYPEER/VERIFYHOST
libcurl defaults may be overridden by environment. Fix: explicit CURLOPT_SSL_VERIFYPEER = 1, CURLOPT_SSL_VERIFYHOST = 2 in make_curl().
H8 — Unbounded lock response buffer
The LOCK response body was accumulated into a growing buffer with no size cap. A malicious server could trigger a multi-gigabyte allocation. Fix: 64 KB cap on the LOCK response accumulation buffer.
M — /proc/self/fd enumeration race — between opendir() and the actual fds used. Not exploitable in practice (child runs with no external interaction at that point). Noted, not fixed.
M — Cache filename collision — URL truncation without hashing. Two URLs that share the same 200-byte prefix map to the same cache file. Known architectural limitation; acceptable for single-user use.
M — Signal-unsafe std::string in lockd — child used std::string in the EOF handler (child_unlock_all). Fixed: replaced with stack-allocated C string buffer.
• Unchecked return values (several) — ftell, chmod, others. Some fixed, some noted.
• World-readable .so in dev builds (cwd search in DEVELOPMENT mode). Noted; acceptable for dev builds.
• Large stack arrays in lock response parser. Fixed: moved to heap with size cap.
• Locale-dependent character comparison in scheme validation. Fixed: explicit ASCII range check.
• Plugin ABI signature / Ed25519 verification — recommended for future work. Not implemented.
• Lock token in child process memory — unavoidable by design; not an issue.
• Cache file not unlinked on failure — leaves partial file in cache. Low risk; next fetch overwrites it.
• Locale issue in scheme string comparison — see Low #4 above.