crypto: accept empty HMAC keys #35688

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:l0rinc/crypto-empty-input-hashing changing 5 files +7 −22
  1. l0rinc commented at 10:46 PM on July 8, 2026: contributor

    Problem: CHMAC_SHA256 and CHMAC_SHA512 copied the key with memcpy(rkey, key, keylen) before zero-padding it. Empty-vector callers can pass a null key pointer, and glibc annotates memcpy arguments as non-null. A minimal Compiler Explorer reproducer shows the unguarded zero-length copy still aborting under current GCC-trunk UBSan, while the guarded variant exits successfully. This is the same class of empty-byte-vector issue discussed for the IPC fuzzer - where std::copy was also considered (see godbolt reproducer above).

    No in-tree caller currently passes a null HMAC key, so this hardens the empty-key boundary for tests and fuzzing.

    Fix: Skip the key copy when keylen == 0. The following memset still zero-fills the whole HMAC key block, so the derived pads and digests are unchanged.

    With the HMAC copy fixed, the crypto fuzzer can pass empty byte vectors directly. The stale eval_script workaround can also be dropped in favor of calling ConsumeRemainingBytes() directly because it already returns before copying when no bytes remain.

  2. crypto: accept empty HMAC keys
    Empty-vector HMAC tests expose a null key pointer in `CHMAC_SHA256` and `CHMAC_SHA512`: empty `std::vector` inputs can return a null `data()` pointer, and glibc annotates `memcpy` arguments as non-null.
    A minimal UBSan reproducer shows the unguarded zero-length copy aborting, while the guarded variant exits successfully: https://godbolt.org/z/jMPP8EhEr
    
    Skip the key copy when `keylen == 0`. The following `memset` still zero-fills the whole HMAC key block, so the derived pads and digests are unchanged.
    
    This is the same class of empty-byte-vector issue discussed for the IPC fuzzer in https://github.com/bitcoin/bitcoin/pull/35118#discussion_r3508842395.
    The IPC discussion also considered `std::copy` after #35010, but the explicit zero-length guard is the smaller fit here and also works for generic byte-span serializers that accept `std::byte` sources (see godbolt above).
    
    With the HMAC copy fixed, the crypto fuzzer can pass empty byte vectors directly.
    The `eval_script` guard is also stale because `ConsumeRemainingBytes()` already returns an empty vector without copying when no bytes remain.
    ac1ddae4f6
  3. DrahtBot added the label Utils/log/libs on Jul 8, 2026
  4. DrahtBot commented at 10:46 PM on July 8, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35688.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. DrahtBot added the label CI failed on Jul 8, 2026
  6. l0rinc closed this on Jul 8, 2026

  7. l0rinc reopened this on Jul 8, 2026

  8. DrahtBot removed the label CI failed on Jul 9, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-09 06:47 UTC