n_signer/audit/REMEDIATION.md

n_signer Security Audit — Remediation Report

Date: 2026-08-13 Scope: Full static security audit of src/, client/, libotppad/, and build configuration Result: 5 findings identified, all remediated and verified

---

Executive Summary

A comprehensive security audit of the n_signer codebase identified 5 security findings across memory safety, network parsing, authentication, and build hardening. All findings have been remediated, code-reviewed, and verified against the existing test suite.

Severity	Count	Status
High	1	✅ Remediated
Medium	3	✅ Remediated
Low	1	✅ Remediated
Total	5	All Fixed

---

Findings and Remediations

F-001: mlock Failure Silently Degraded to Pageable Memory

Severity	Medium
File	src/secure_mem.c:762–798
Status	✅ Remediated

Problem. secure_buf_alloc() called mlock() to pin secret material (mnemonic, private keys) in RAM. On failure (e.g., RLIMIT_MEMLOCK exhausted, missing CAP_IPC_LOCK), it printed a warning and returned success with the buffer unlocked. No caller checked buf->locked, so the process continued with secrets in pageable memory — silently undermining the "crash = total wipe" and "no filesystem footprint" guarantees. An attacker with disk access after the fact could recover key material from swap.

Fix. mlock failure is now fatal by default. The function prints a diagnostic with strerror(errno) and returns -1, causing startup to abort. A new opt-in escape hatch, secure_buf_allow_unlocked(), is wired to the --allow-unlocked-memory CLI flag in src/main.c:3653 for development/container environments where mlock is unavailable.

Files changed:

• src/secure_mem.c — fatal-by-default logic, secure_buf_allow_unlocked(), added <errno.h>
• src/main.c — --allow-unlocked-memory argument parsing + declaration

---

F-002: HTTP Content-Length Parsed with atol() — No Error Detection

Severity	Medium
File	src/http_listener.c:75–163
Status	✅ Remediated

Problem. The HTTP request parser read Content-Length using atol(), which has no error detection: it returns 0 for non-numeric input (indistinguishable from a real 0), and silently truncates values exceeding LONG_MAX. The value was stored in a signed long and compared against a size_t limit, creating signed/unsigned confusion. A Content-Length near LONG_MAX could trigger a giant allocation attempt (DoS via OOM or NULL-deref crash).

Fix. Replaced atol() with strtoull() and full validation:

• Rejects empty/non-numeric values (endptr == p)
• Rejects trailing garbage (only whitespace/CR allowed after digits)
• Rejects values exceeding SIZE_MAX
• Changed content_length from long to size_t, eliminating signed/unsigned confusion
• Added a has_content_length flag to distinguish "missing header" from "zero length"

Files changed:

• src/http_listener.c — safe parsing, type fix, drain loop type fix

---

F-003: Auth Envelope Nonce Cache Replay After Wrap

Severity	High
Files	src/auth_envelope.h, src/auth_envelope.c
Status	✅ Remediated

Problem. Replay protection used a bounded FIFO cache of 1024 event IDs (AUTH_NONCE_CACHE_SIZE:10). When full, the oldest entry was evicted (circular overwrite). An attacker who captured 1024 valid auth envelopes could replay any of them after the cache wrapped — the evicted nonce would no longer be detected as a duplicate. Combined with the 30-second timestamp skew window, this allowed impersonation of any previously-seen caller.

Fix. Replaced the bounded FIFO cache with a hybrid per-pubkey replay tracker:

1. Monotonic timestamp per pubkey — tracks the highest created_at seen for each of up to 64 pubkeys. Any envelope with created_at < max_seen is rejected as a replay. This has no wrap-around weakness.
2. Event ID set for the current second — because created_at has 1-second granularity, a per-(pubkey, second) set of up to 32 event IDs allows multiple legitimate concurrent requests within the same second while still rejecting exact duplicates.
3. When created_at > max_seen, the event ID set is cleared and the timestamp advances.

The initial monotonic-only version was caught by the existing test suite (tests/test_auth_envelope.c) which builds multiple same-second requests — the hybrid design passes all 13 tests.

Files changed:

• src/auth_envelope.h — new auth_pubkey_entry_t structure with max_created_at + event_ids[]
• src/auth_envelope.c — new auth_nonce_cache_check_and_update() implementing the hybrid check; event ID extracted from the signed envelope's id field

---

F-004: OTP Binary Header Checksum Parsed with sscanf — Return Value Ignored

Severity	Low
File	src/otp_pad.c:408–416
Status	✅ Remediated

Problem. When building a binary .otp output header, the hex pad checksum was converted to bytes using sscanf("%02x") in a loop, but the return value was never checked. If the checksum string were ever malformed, sscanf would leave the destination variable uninitialized, producing garbage in the output header.

Fix. Added a return-value check: if sscanf does not return exactly 1, the function zeroizes the scratch buffer and returns an error. (An earlier version of this fix incorrectly called free(blob) before blob was declared — this was caught in code review and corrected.)

Files changed:

• src/otp_pad.c — sscanf return value checked with proper error cleanup

---

F-005: Missing Compiler Hardening Flags

Severity	Medium
File	Makefile:1–10
Status	✅ Remediated

Problem. The build used only -Wall -Wextra -Os with no exploit-mitigation flags. The resulting binary had no stack canaries, no _FORTIFY_SOURCE bounds checking, no PIE (fixed load address — trivial ROP), writable GOT (no RELRO), and a potentially executable stack. For a program that parses untrusted network input while holding signing keys, these gaps significantly raise the impact of any memory-corruption bug.

Fix. Added the standard hardening flag set to CFLAGS and LDFLAGS:

• -fstack-protector-strong — stack canaries
• -D_FORTIFY_SOURCE=2 — compile-time + runtime bounds checking for libc functions
• -fPIE / -pie — position-independent executable (ASLR for code)
• -Wl,-z,relro -Wl,-z,now — full RELRO (read-only GOT after startup)
• -Wl,-z,noexecstack — non-executable stack (NX)
• -fstack-clash-protection — stack-clash probing

Verification. The rebuilt binary is confirmed as ELF 64-bit LSB pie executable. Notably, the new -fstack-protector-strong flag immediately caught a pre-existing latent buffer overflow in tests/test_selector.c (stack smashing detected at runtime) — a bug that was previously silent. This validates the value of the hardening flags.

Files changed:

• Makefile — hardening flags in CFLAGS and LDFLAGS

---

Post-Remediation Defects Caught in Review

During code review of the initial fixes, 4 defects were identified and corrected before final verification:

#	Defect	File	Resolution
1	errno used without #include <errno.h> (compile error)	src/secure_mem.c	Added include
2	free(blob) referenced before blob was declared (compile error)	src/otp_pad.c	Removed erroneous free(); only secure_memzero needed on that path
3	Duplicated pubkey validation block (dead code)	src/auth_envelope.c	Removed duplicate
4	secure_buf_allow_unlocked() not declared in main.c's headerless block (compile error)	src/main.c	Added declaration
5	Monotonic-only timestamp rejected same-second requests (test failure)	src/auth_envelope.c	Upgraded to hybrid timestamp + event-ID design

---

Verification

Build

• Compiles cleanly with all hardening flags enabled
• Output binary confirmed as PIE: ELF 64-bit LSB pie executable, x86-64

Test Suite

Test	Result
test_auth_envelope	✅ 13/13 passed (validates F-003 hybrid design)
test_mnemonic	✅ All passed
test_role_table	✅ All passed
test_enforcement	✅ 10/10 passed
test_policy	✅ 43/43 passed
test_socket_name	✅ All passed
test_mnemonic_input	✅ All passed
test_path_whitelist	✅ 40/41 (1 pre-existing failure, unrelated)
test_selector	⚠️ Stack smashing detected — hardening caught a pre-existing latent bug (unrelated to remediations)

---

Remaining Recommendations (Non-Blocking)

These items were noted during the audit but are not security findings:

1. Fix the latent test_selector buffer overflow now exposed by -fstack-protector-strong. This is a pre-existing bug in the test code, not in production code.
2. Apply the same hardening flags to the musl-static build in Dockerfile.alpine-musl / build_static.sh (verify musl-gcc supports -fstack-clash-protection, GCC 8+).
3. Pin vendored dependency versions (cJSON, nostr_core_lib, PQClean, secp256k1) to specific commits and track known CVEs.
4. Add fuzz testing for the HTTP parser and transport frame parser.
5. Document the --allow-unlocked-memory flag in the README security section.

---

Files Changed Summary

File	Finding(s)
src/secure_mem.c	F-001
src/main.c	F-001 (flag wiring)
src/http_listener.c	F-002
src/auth_envelope.h	F-003
src/auth_envelope.c	F-003
src/otp_pad.c	F-004
Makefile	F-005