Follow-up to #1579. buf still holds the nonce or its negation, so ought to be cleared.
schnorrsig: Securely clear buf containing k or its negation #1731
pull john-moffett wants to merge 1 commits into bitcoin-core:master from john-moffett:schnorr-clear-buf changing 1 files +4 −3-
john-moffett commented at 1:56 PM on September 2, 2025: contributor
- real-or-random approved
-
real-or-random commented at 2:47 PM on September 2, 2025: contributor
utACK 366b125eadaaffc48260791f09497b55c737cafb
It's a bit crazy that we clear
rjbecause the non-uniqueness of the Jacobian representation could leak information aboutkbut we forgot the damn buffer wherekhas been serialized.Love that someone is reading the code. I take a closer look occasionally when I need to convince myself that it is correct, but it's not a very systematic effort.
- real-or-random added the label bug on Sep 2, 2025
- real-or-random added the label side-channel on Sep 2, 2025
-
real-or-random commented at 2:50 PM on September 2, 2025: contributor
@theStack Want to review this? :)
- theStack approved
-
theStack commented at 3:43 PM on September 2, 2025: contributor
Code-review ACK 366b125eadaaffc48260791f09497b55c737cafb
Nice find, and a bit surprising indeed that this was missed :grimacing: nit: as this buffer is only used for the serialization of the nonce, we could rename it to reflect that, e.g. to
nonce_buf(fwiw, in the ECDSA signing routine the namenonce32is used) -
325d65a8cf
Rename and clear var containing k or -k
buf currently holds k or -k and isn't cleared, so clear it and rename to nonce32 to clarify its sensitivity and match how it is named in the corresponding ECDSA sign_inner.
-
john-moffett commented at 4:46 PM on September 2, 2025: contributor
we could rename it to reflect that, e.g. to
nonce_buf(fwiw, in the ECDSA signing routine the namenonce32is used)Good idea, as it makes it clearer that it's sensitive information, too.
- john-moffett force-pushed on Sep 2, 2025
- theStack approved
-
theStack commented at 7:03 PM on September 2, 2025: contributor
Code review re-ACK 325d65a8cfae6d4c34098709d0a9e942e4963d03
- real-or-random approved
-
real-or-random commented at 8:37 PM on September 2, 2025: contributor
utACK 325d65a8cfae6d4c34098709d0a9e942e4963d03
- real-or-random merged this on Sep 2, 2025
- real-or-random closed this on Sep 2, 2025
- vmta referenced this in commit 2b25f561a0 on Sep 21, 2025
- fanquake referenced this in commit 42c7d35d3a on Oct 14, 2025
- fanquake referenced this in commit 3cbf7cb3e6 on Oct 15, 2025
- luke-jr referenced this in commit 3594134e4b on Jan 29, 2026
- Sjors referenced this in commit d5660d3a13 on Feb 16, 2026
- real-or-random referenced this in commit 42ae776d3b on Feb 25, 2026
- github-actions[bot] referenced this in commit c3f80fff5f on Mar 1, 2026
- github-actions[bot] referenced this in commit 758d4e90b4 on Mar 1, 2026
- github-actions[bot] referenced this in commit 4aeff8400e on Mar 1, 2026
- github-actions[bot] referenced this in commit 68a2178f22 on Mar 1, 2026
- github-actions[bot] referenced this in commit a8bc1a0b2b on Mar 1, 2026
- github-actions[bot] referenced this in commit 5f15eb0c55 on Mar 1, 2026
- 0x000000000019d6689c085ae165831e934ff76 referenced this in commit d54574beca on Mar 2, 2026
- 0x000000000019d6689c085ae165831e934ff76 referenced this in commit 3b9450150d on Mar 2, 2026
- csjones referenced this in commit fb3e16af04 on Mar 2, 2026
- csjones referenced this in commit a4d92824ae on Mar 2, 2026
