hash: Use size_t instead of int for RFC6979 outlen copy #1729

1. 
   john-moffett commented at 2:40 pm on September 1, 2025: contributor

   If outlen > INT_MAX it results in segfault or hang (when outlen is a multiple of 2^32) on most implementations due to conversion in: int now = outlen producing negative values or zero. Unreachable in current code and highly improbable in future practice, but fits contract better and fixes a couple of compiler warnings.
2. Use size_t instead of int for RFC6979 outlen copy

   If outlen is > INT_MAX, could trigger segfault or hang after copy
   int now = outlen.

   960ba5f9c6
3. 
   fanquake commented at 2:42 pm on September 1, 2025: member

   fixes a couple of compiler warnings

   Which compiler / warnings?

4. 
   john-moffett commented at 2:53 pm on September 1, 2025: contributor

   Not with default flags. I used clang -Wimplicit-int-conversion -Wshorten-64-to-32. It’d probably show up with gcc -Wconversion. There are a lot of benign warnings, but this one stood out to me.
5. in src/hash_impl.h:1 in 960ba5f9c6

   ---

   ---

   

   real-or-random commented at 3:28 pm on September 1, 2025:

   If you want to improve this further, I’d suggest

   • renaming now to chunk_len (consistency with the SHA256 implementation above)
   • moving the entire if block just below the initialization of now (or chunk_len). (The goal is to compute max(outlen, 32). Spreading this over two blocks is a bit silly. It’s probably a result of the C89 rule that declarations need to be at the beginning of a block. edit: Indeed, see https://github.com/bitcoin-core/secp256k1/commit/792bcdb015b0272928d4dd602557faa685dcd5a1#diff-e91b67aa33d606267c28ce5da48a89cc789f4081c176eb581a9bff780afd1d6cR260-L262 )

   ---

   

   john-moffett commented at 1:53 pm on September 2, 2025:

   Thanks! I’ll hold off for the moment. Hopefully it’ll inspire someone to replace this nonce generation process with something more lightweight and straightforward like BIP340’s approach. :)
6. real-or-random approved
7. 
   real-or-random commented at 3:30 pm on September 1, 2025: contributor

   utACK 960ba5f9c60c3fd454ad4160ce9131eb77d61d7e

   Thanks a lot! Consistency of integer types is indeed a bit of a weak spot, at least in the old parts of the library code.

8. real-or-random added the label assurance on Sep 1, 2025
9. real-or-random added the label tweak/refactor on Sep 1, 2025
10. theStack approved
11. 
    theStack commented at 6:05 pm on September 2, 2025: contributor

    Code-review ACK 960ba5f9c60c3fd454ad4160ce9131eb77d61d7e
12. real-or-random merged this on Sep 2, 2025
13. real-or-random closed this on Sep 2, 2025

    ---

Contributors
john-moffett fanquake real-or-random theStack

Labels
assurance tweak/refactor