From 94000967f6dc05b1afd435e79b1bbc597e29f816 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 8 Sep 2026 19:00:00 +0200 Subject: [PATCH 1/5] sigcache: harden proof cache keys with length-prefixed hashing Switch range-proof and surjection-proof cache hashers from raw CSHA256 concatenation to CHashWriter (SER_GETHASH). This serializes each field with a length prefix, so distinct argument tuples with byte-identical raw concatenations no longer collide to the same cache key. A cache entry is a positive verification result; a collision would let an attacker bypass verification. The fix affects two caches: - ComputeEntryRangeProof: proof, commitment, asset_commitment, scriptPubKey - ComputeEntrySurjectionProof: add vTags to key (was missing entirely) Both caches retain the per-process 64-byte salted midstate (nonce || PADDING_RANGE_PROOF / PADDING_SURJECTION_PROOF) for domain separation between the two proof types. Expose thin test-only hooks (TestComputeEntryRangeProof / TestComputeEntrySurjectionProof) so unit tests can reach the anonymous-namespace cache internals. --- src/script/sigcache.cpp | 87 ++++++++++++++++++++++++++++++++--------- src/script/sigcache.h | 14 +++++++ 2 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp index 9f7bb9592b5..96e2e3dd5ca 100644 --- a/src/script/sigcache.cpp +++ b/src/script/sigcache.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -26,23 +27,23 @@ namespace { class CSignatureCache { private: - //! Entries are SHA256(nonce || 'E' or 'S' || 31 zero bytes || signature hash || public key || signature): + //! Salted SHA256 midstates, domain-separated by signature or proof type. CSHA256 m_salted_hasher_ecdsa; CSHA256 m_salted_hasher_schnorr; - CSHA256 m_salted_hasher_range_proof; - CSHA256 m_salted_hasher_surjection_proof; + CHashWriter m_salted_hasher_range_proof; + CHashWriter m_salted_hasher_surjection_proof; typedef CuckooCache::cache map_type; map_type setValid; std::shared_mutex cs_sigcache; public: - CSignatureCache() + CSignatureCache(): + m_salted_hasher_range_proof(SER_GETHASH,0), + m_salted_hasher_surjection_proof(SER_GETHASH,0) { uint256 nonce = GetRandHash(); - // We want the nonce to be 64 bytes long to force the hasher to process - // this chunk, which makes later hash computations more efficient. We - // just write our 32-byte entropy, and then pad with 'E' for ECDSA and - // 'S' for Schnorr (followed by 0 bytes). + // Use 64-byte, type-specific salted midstates so later hash computations + // can start after the first SHA256 chunk. static constexpr unsigned char PADDING_ECDSA[32] = {'E'}; static constexpr unsigned char PADDING_SCHNORR[32] = {'S'}; static constexpr unsigned char PADDING_RANGE_PROOF[32] = {'r'}; @@ -51,10 +52,8 @@ class CSignatureCache m_salted_hasher_ecdsa.Write(PADDING_ECDSA, 32); m_salted_hasher_schnorr.Write(nonce.begin(), 32); m_salted_hasher_schnorr.Write(PADDING_SCHNORR, 32); - m_salted_hasher_range_proof.Write(nonce.begin(), 32); - m_salted_hasher_range_proof.Write(PADDING_RANGE_PROOF, 32); - m_salted_hasher_surjection_proof.Write(nonce.begin(), 32); - m_salted_hasher_surjection_proof.Write(PADDING_SURJECTION_PROOF, 32); + m_salted_hasher_range_proof << nonce << PADDING_RANGE_PROOF; + m_salted_hasher_surjection_proof << nonce << PADDING_SURJECTION_PROOF; } void @@ -72,13 +71,39 @@ class CSignatureCache } // ELEMENTS: - void ComputeEntryRangeProof(uint256& entry, const std::vector& proof, const std::vector& commitment, const std::vector& asset_commitment, const CScript& scriptPubKey) { - CSHA256 hasher = m_salted_hasher_range_proof; - hasher.Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Write(asset_commitment.data(), asset_commitment.size()).Write(scriptPubKey.data(), scriptPubKey.size()).Finalize(entry.begin()); + void ComputeEntryRangeProof(uint256& entry, + const std::vector& proof, + const std::vector& commitment, + const std::vector& asset_commitment, + const CScript& script_pub_key) const + { + CHashWriter hasher = m_salted_hasher_range_proof; + // We commit to both commitments and the scriptPubKey because these are + // committed to by the rangeproof itself; a change in any of them would + // invalidate the proof. Since these are exactly the arguments to + // CachingRangeProofChecker::VerifyRangeProof (below), there is no + // additional data that could affect the rangeproof's validity. + // Serialization length-prefixes every field, including the variable-length + // proof and script, so distinct argument tuples cannot share an encoding. + hasher << proof << commitment << asset_commitment << script_pub_key; + entry = hasher.GetSHA256(); } - void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector& proof, const std::vector& commitment) { - CSHA256 hasher = m_salted_hasher_surjection_proof; - hasher.Write(hash.begin(), 32).Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Finalize(entry.begin()); + void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector& proof, const std::vector& commitment, const std::vector& vTags) const { + CHashWriter hasher = m_salted_hasher_surjection_proof; + // We hash all arguments passed to CachingSurjectionProofChecker::VerifySurjectionProof, + // to ensure that any change in the way that the verification function is called will + // trigger a cache miss and explicit verification. However, we note that the `wtxid` + // (hash) commits to all the other data such that we could technically hash only it. + // We retain the other data as a defense against future refactorings. + // + // Serialize vTags as a flat byte vector (each secp256k1_generator is 64 bytes). + std::vector vTagsBytes; + vTagsBytes.reserve(vTags.size() * 64); + for (const auto& tag : vTags) { + vTagsBytes.insert(vTagsBytes.end(), std::begin(tag.data), std::end(tag.data)); + } + hasher << hash << proof << commitment << vTagsBytes; + entry = hasher.GetSHA256(); } bool @@ -175,6 +200,11 @@ void InitSurjectionproofCache() bool CachingRangeProofChecker::VerifyRangeProof(const std::vector& vchRangeProof, const std::vector& vchValueCommitment, const std::vector& vchAssetCommitment, const CScript& scriptPubKey, const secp256k1_context* secp256k1_ctx_verify_amounts) const { + // ELEMENTS: NOTE FOR FUTURE EDITORS: every argument to this function that + // carries data (i.e. everything except the secp256k1 context, which is + // stateless) MUST be included in ComputeEntryRangeProof. Omitting any + // argument risks returning a cached positive result for a proof that was + // verified with different inputs. uint256 entry; rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey); @@ -227,7 +257,7 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr // wtxid commits to all data including surj targets // we need to specify the proof and output asset point to be unique uint256 entry; - surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector(std::begin(gen.data), std::end(gen.data))); + surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector(std::begin(gen.data), std::end(gen.data)), vTags); if (surjectionProofCache.Get(entry, !store)) { return true; @@ -244,5 +274,24 @@ bool CachingSurjectionProofChecker::VerifySurjectionProof(secp256k1_surjectionpr return true; } +// Test-only hooks (see sigcache.h). Forward to the anonymous-namespace caches. +void TestComputeEntryRangeProof(uint256& entry, + const std::vector& proof, + const std::vector& commitment, + const std::vector& asset_commitment, + const CScript& script_pub_key) +{ + rangeProofCache.ComputeEntryRangeProof(entry, proof, commitment, asset_commitment, script_pub_key); +} + +void TestComputeEntrySurjectionProof(uint256& entry, + const uint256& hash, + const std::vector& proof, + const std::vector& commitment, + const std::vector& vTags) +{ + surjectionProofCache.ComputeEntrySurjectionProof(entry, hash, proof, commitment, vTags); +} + // END ELEMENTS // diff --git a/src/script/sigcache.h b/src/script/sigcache.h index 0c6477edec7..e91bded9c94 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -70,6 +70,20 @@ class CachingSurjectionProofChecker void InitRangeproofCache(); void InitSurjectionproofCache(); +// Test-only hooks: expose the (anonymous-namespace) cache-entry computation so +// unit tests can verify collision-resistance and domain separation. These are +// NOT part of the consensus/validation API and are only used by unit tests. +void TestComputeEntryRangeProof(uint256& entry, + const std::vector& proof, + const std::vector& commitment, + const std::vector& asset_commitment, + const CScript& script_pub_key); +void TestComputeEntrySurjectionProof(uint256& entry, + const uint256& hash, + const std::vector& proof, + const std::vector& commitment, + const std::vector& vTags); + // END ELEMENTS // From 1513d63d17bbbb2373640ead8ac0215c749281db Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 8 Sep 2026 19:00:14 +0200 Subject: [PATCH 2/5] sigcache: add proof cache key collision-resistance tests Test the field-boundary, script-sensitivity, domain-separation, determinism, and vTags-sensitivity properties of the range-proof and surjection-proof cache entry computation. --- src/Makefile.test.include | 1 + src/test/sigcache_tests.cpp | 164 ++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 src/test/sigcache_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 24631f8450f..7b0ae72b0ab 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -132,6 +132,7 @@ BITCOIN_TESTS =\ test/serialize_tests.cpp \ test/settings_tests.cpp \ test/sighash_tests.cpp \ + test/sigcache_tests.cpp \ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/sock_tests.cpp \ diff --git a/src/test/sigcache_tests.cpp b/src/test/sigcache_tests.cpp new file mode 100644 index 00000000000..6cd13c5fbbd --- /dev/null +++ b/src/test/sigcache_tests.cpp @@ -0,0 +1,164 @@ +// Copyright (c) 2026 The Elements developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +// +// Tests for the Elements proof-cache entry computation (script/sigcache.cpp). +// +// These tests guard the collision-resistance and domain-separation properties +// of the cache keys used for the range-proof and surjection-proof caches. +// A cache entry is a *positive* verification result, so a key collision means +// accepting a proof without ever verifying it. The keys are computed with +// CHashWriter serialization, which length-prefixes every field, so two +// distinct argument tuples must never produce the same cache entry, and the +// two proof types must live in disjoint key spaces (domain separation). + +#include