Skip to content

[ Security ] - keep key material off disk, fix block-length parsing UB - #13

Merged
cristiancmoises merged 1 commit into
cabelo:mainfrom
cristiancmoises:security-harden-key-handling
Jun 16, 2026
Merged

[ Security ] - keep key material off disk, fix block-length parsing UB#13
cristiancmoises merged 1 commit into
cabelo:mainfrom
cristiancmoises:security-harden-key-handling

Conversation

@cristiancmoises

Copy link
Copy Markdown
Collaborator

Security fixes:

  • Stop staging keys in predictable /tmp paths. The in-memory encrypt/decrypt path wrote the public key (and, worse, the private key) to /tmp/zupt_{pub,priv}_ just to hand a path to the file-based init functions. That was a symlink-attack vector and it defeated the library's own mlock protection by leaving private key material on disk. Added zupt_hybrid_encrypt_init_mem / zupt_hybrid_decrypt_init_mem, which parse the key straight from the caller's buffer; the C wrapper now uses them and never touches disk.

  • Create private key files with 0600 instead of the umask default. Applied to both the C path (zupt_hybrid_keygen, via open+fdopen) and the C++ path (KeyGenerator::saveKeyPair, via open+fchmod before the ofstream), so a freshly generated key is never briefly world-readable.

  • Fix sign-extension/UB when parsing the per-block payload length on decrypt. Each byte promoted to int, so byte << 24 is undefined and sign-extends when the top bit is set, yielding a bogus huge block_len. Cast each byte to size_t before shifting.

Cleanup:

  • Remove dead, incorrect zupt_hybrid_derive_keys (it ignored the X25519 shared secret and the ml_pk argument, so it produced keys the decrypt side could never reproduce) and the unused zupt_hybrid_decrypt_derive_keys.

Refactored the file-based init functions to share an encaps/decaps core with the new in-memory variants, so there is no duplicated KDF logic. Build is clean (no warnings) and all five test suites pass.

Security fixes:

- Stop staging keys in predictable /tmp paths. The in-memory
  encrypt/decrypt path wrote the public key (and, worse, the private
  key) to /tmp/zupt_{pub,priv}_<pid> just to hand a path to the
  file-based init functions. That was a symlink-attack vector and it
  defeated the library's own mlock protection by leaving private key
  material on disk. Added zupt_hybrid_encrypt_init_mem /
  zupt_hybrid_decrypt_init_mem, which parse the key straight from the
  caller's buffer; the C wrapper now uses them and never touches disk.

- Create private key files with 0600 instead of the umask default.
  Applied to both the C path (zupt_hybrid_keygen, via open+fdopen) and
  the C++ path (KeyGenerator::saveKeyPair, via open+fchmod before the
  ofstream), so a freshly generated key is never briefly world-readable.

- Fix sign-extension/UB when parsing the per-block payload length on
  decrypt. Each byte promoted to int, so `byte << 24` is undefined and
  sign-extends when the top bit is set, yielding a bogus huge block_len.
  Cast each byte to size_t before shifting.

Cleanup:

- Remove dead, incorrect zupt_hybrid_derive_keys (it ignored the X25519
  shared secret and the ml_pk argument, so it produced keys the decrypt
  side could never reproduce) and the unused zupt_hybrid_decrypt_derive_keys.

Refactored the file-based init functions to share an encaps/decaps core
with the new in-memory variants, so there is no duplicated KDF logic.
Build is clean (no warnings) and all five test suites pass.
@cristiancmoises cristiancmoises self-assigned this Jun 13, 2026
@cristiancmoises
cristiancmoises merged commit 021ba1a into cabelo:main Jun 16, 2026
4 checks passed
@cabelo

cabelo commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants