16 findings: 0 Critical, 4+ High, 6 Medium, 4 Low, 2 Info. All critical issues already fixed after audit 1. All new findings fixed.
All 7 o3 Critical+High fixes confirmed correct: binary IPC protocol, cache 0700, single-fd dlopen, PROTOCOLS_STR+FOLLOWLOCATION=0, RFC 3986 scheme validation, SSL VERIFYPEER/VERIFYHOST, 64 KB lock response cap.
H9 — recv_string unbounded allocation
The child allocated a std::string of len bytes before checking the value of len. A corrupted or malicious frame could cause a multi-gigabyte allocation, crashing the daemon and leaving the server lock held.
Fix: if (len > max_len) return false before the allocation. Constants: MAX_IPC_URL = 8192, MAX_IPC_PATH = 4096.
H — DEVELOPMENT builds load plugins from CWD
In DEVELOPMENT builds, the cwd is searched for plugins. If a developer runs pwsafe from a world-writable directory, a malicious pwsafe-https.so could be loaded. Accepted risk for development builds; production builds never search cwd.
H — Plugin lacks signature verification
Plugins are verified only by the identity string in .comment (obfuscation, not authentication). A build-time Ed25519 signature would prevent loading of tampered plugins. Noted as future work.
H — lockd_release not strictly async-signal-safe
pws_lockd_release() calls write() (async-signal-safe) but also reads the url string from the lock registry, which involves std::string member access. In practice the write() dominates and the signal window is tiny, but technically not provably safe. Noted; acceptable given the constraints.
M10 — No EINTR handling in send_all/recv_all
write()/read() can return -1 with EINTR on a signal; the loop would exit early, leaving a partially written frame and desynchronising the protocol.
Fix: if (errno == EINTR) continue; in both send_all and recv_all.
M13 — OPTIONS probe missing curl hardening
server_supports_locking() used curl_easy_init() directly instead of make_curl(), bypassing protocol restrictions and SSL verification. A malicious redirect or downgrade could affect the OPTIONS probe.
Fix: now uses make_curl(url).
M7 — Cache file created with fopen (inherits umask)
webdav_fetch created the cache file with fopen("wb") — typically producing 0644 permissions, readable by any local user.
Fix: open(O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0600) + fdopen().
M2 — Plugin fd missing S_ISREG check
so_claims_scheme_fd checked fstat return code but not that the file was a regular file. A FIFO or character device would pass the check.
Fix: if (!S_ISREG(st.st_mode)) return false;
M6 — Cache filename collision
Two URLs that share the same sanitised-truncated prefix map to the same cache file. Known limitation — acceptable for single-user single-database use. Documented.
L14 — ftell() return value unchecked
ftell() returns -1 on a non-seekable stream. Passing that to CURLOPT_INFILESIZE_LARGE sends a garbage size to the server.
Fix: check fsize < 0 → return EIO.
L15 — Lock-Token / DAV: header case-sensitive
RFC 7230 §3.2 requires case-insensitive header name matching. The Lock-Token: extraction and DAV: check both used strncmp (case-sensitive).
Fix: replaced with strncasecmp.
L16 — unlock dead code
The plugin's unlock() function had an unreachable code path that called t->unlock() a second time after already returning. Removed.