#35688 crypto: accept empty HMAC keys

ReviewabilityReviewsAgreementSize
#35688 l0rinc crypto: accept empty HMAC keysAuthor silent 115d1Positive4 + 5 tests
  • Avoid undefined behavior and UBSan warnings when initializing HMAC with empty keys
  • Remove defensive workarounds in fuzz targets for empty byte vectors
  • Author has been silent for 77 days, so responses to review feedback may be delayed
  • Concept approval without stated reasons (sedited)
  • Resolved concern on memcpy semantics and safety by switching to std::copy (maflcko)
  • 4 lines added or modified outside tests
  • 5 lines added or modified in tests
  • 22 lines removed in total
  • 5 files, 2 commits
Utilities (logging, arguments, libraries)#23 of 59
P3 · bug fix
  • P3 because it fixes minor undefined behavior in crypto utilities without direct user-facing impact
  • Resolves UBSan warnings on zero-length keys and cleans up defensive checks in fuzz targets
  • Ranking pass: Moved up: undefined behaviour in HMAC init belongs with the other crypto-primitive correctness fixes.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Replaces memcpy with std::copy in CHMAC_SHA256 and CHMAC_SHA512 so that zero-length HMAC keys do not pass a null pointer to memcpy and trigger UBSan warnings. It adds empty-key test vectors to crypto_tests and removes empty-input workarounds from the crypto and eval_script fuzz targets.

Problem: Calling memcpy with a null pointer even for zero bytes is undefined behavior in C++, triggering UBSan runtime warnings and forcing fuzz targets to add defensive checks against empty byte vectors.

Reviewability

Stale · Author silent 115d

No author activity for 115 days; a review may wait a long time for a response.

Agreement

Positive

support without stated reasons, no open objection (sedited)

Thread notes: Initial record. maflcko suggested using std::copy to handle empty source ranges, which was implemented in a subsequent push; sedited provided an ACK.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)objection, approachmemcpy does not have defined semantics for empty source ranges in all C++ versions and lacks type safety compared to std::copyfixed · fix pushed · v1, not yet re-checkedansweredyes2026-07-09: 'Why not use `std::copy`? It has several benefits'
Status from: 2026-07-09: l0rinc: 'I switched both HMAC copies to `std::copy`'

Support:

  • sedited (member) (ACK, 2026-09-04): (no reason given) [not substantive]

Participants:

  • maflcko (member, objection, 4 statements 2026-07-09 to 2026-07-09): suggested std::copy for defined semantics on empty ranges, which author adopted
  • sedited (member, support, 1 statements 2026-09-04 to 2026-09-04): ACKed the PR

Files

25 lines under test/bench/ci.

  • src/test/fuzz/eval_script.cpp +1/-10
  • src/test/fuzz/crypto.cpp +0/-10
  • src/test/crypto_tests.cpp +4/-0
  • src/crypto/hmac_sha256.cpp +2/-1
  • src/crypto/hmac_sha512.cpp +2/-1

Card

The short text the ranking pass compares across PRs.

This PR replaces memcpy with std::copy in CHMAC_SHA256 and CHMAC_SHA512 to avoid undefined behavior and UBSan null-pointer warnings when initializing HMAC with empty keys. It adds empty-key HMAC test vectors and removes unnecessary empty-input guards in the crypto and eval_script fuzz targets. The change improves the correctness of cryptographic primitives under sanitizers and enables cleaner fuzz coverage of edge cases. The code has an approval from sedited after addressing suggestions from maflcko, though the author has been inactive for over 60 days.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:35, confidence high, input hash 94cf1c2ee4123484
thread read through 2026-09-04 (7 statements), head dc67c4cc39
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Utils/log/libs

Categories

Utilities (logging, arguments, libraries)

P3 · bug fix (ranking pass: Moved up: undefined behaviour in HMAC init belongs with the other crypto-primitive correctness fixes.)

Fixes undefined behavior in crypto primitives triggered by null pointers with zero-length keys. Replacing memcpy with std::copy resolves UBSan warnings and allows removal of defensive workarounds in fuzz targets.

Membership: Matches the Utils/log/libs label and modifies cryptographic primitives in src/crypto/hmac_sha256.cpp and src/crypto/hmac_sha512.cpp.

Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 1