← Watch feed
sigcache: harden proof cache keys with length-prefixed hashing
Public commit record
83/100 · Strong
What the developer wrote
Authored by Byron Hambly
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.
(cherry picked from commit 94000967f6dc05b1afd435e79b1bbc597e29f816)
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit fixes a cache-key collision bug in Elements' proof verification caches. Previously, the cache key for range proofs and surjection proofs was built by simply concatenating raw bytes, so two different sets of inputs could accidentally produce the same key. Because a cache hit means 'this proof was already verified, skip checking it again,' an attacker who could trigger such a collision could potentially make the node accept an invalid proof. The fix switches to length-prefixed serialization so distinct inputs can no longer collide, and also adds a missing input (vTags) to the surjection-proof cache key. The commit message explicitly calls this a hardening fix against verification bypass.
Recommended action
Treat this as a security hardening fix and include it in the next maintenance release. Add or review unit tests using the new TestComputeEntryRangeProof/TestComputeEntrySurjectionProof hooks to verify that distinct inputs produce distinct cache keys and that domain separation between range and surjection caches holds. Audit any other caches in the codebase that still use raw concatenation for key construction.
Security signals we found
01
Cache-key collision in proof verification cache could allow bypass of cryptographic verification
02
Raw concatenation of variable-length fields in cache-key construction
03
Missing input (vTags) in surjection-proof cache key
04
Fix uses length-prefixed serialization (CHashWriter/SER_GETHASH) to ensure distinct tuples produce distinct keys
05
Commit message explicitly states 'A cache entry is a positive verification result; a collision would let an attacker bypass verification'
Evidence from the diff
The patch changes ComputeEntryRangeProof and ComputeEntrySurjectionProof in src/script/sigcache.cpp from raw CSHA256.Write() concatenation to CHashWriter (SER_GETHASH). CHashWriter prefixes each serialized field with its length, eliminating collisions between argument tuples whose raw concatenation is byte-identical but semantically distinct. It also adds vTags to the surjection-proof cache key, which was previously omitted entirely. The salted midstate initialization is updated to use HashWriter for the proof caches while keeping the 64-byte per-process nonce+padding domain separation. Test-only hooks are exposed to allow unit tests to exercise the cache-entry functions directly.
Changed components
src/script/sigcache.cppsrc/script/sigcache.hSignatureCache::ComputeEntryRangeProofSignatureCache::ComputeEntrySurjectionProofCachingRangeProofChecker::VerifyRangeProofCachingSurjectionProofChecker::VerifySurjectionProofInspect captured patch +92 / −28
### src/script/sigcache.cpp
@@ -6,6 +6,7 @@
#include <script/sigcache.h>
#include <crypto/sha256.h>
+#include <hash.h>
#include <logging.h>
#include <pubkey.h>
#include <random.h>
@@ -20,10 +21,8 @@
SignatureCache::SignatureCache(const size_t max_size_bytes)
{
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'};
@@ -32,10 +31,8 @@ SignatureCache::SignatureCache(const size_t max_size_bytes)
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;
const auto [num_elems, approx_size_bytes] = setValid.setup_bytes(max_size_bytes);
LogPrintf("Using %zu MiB out of %zu MiB requested for signature cache, able to store %zu elements\n",
@@ -55,13 +52,41 @@ void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Sp
}
// ELEMENTS:
-void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const {
- 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 SignatureCache::ComputeEntryRangeProof(uint256& entry,
+ const std::vector<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<unsigned char>& asset_commitment,
+ const CScript& script_pub_key) const
+{
+ HashWriter 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 SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const {
- 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 SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const
+{
+ HashWriter 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<unsigned char> 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 SignatureCache::Get(const uint256& entry, const bool erase)
@@ -130,6 +155,11 @@ bool InitSurjectionproofCache(size_t max_size_bytes)
bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>& vchRangeProof, const std::vector<unsigned char>& vchValueCommitment, const std::vector<unsigned char>& 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);
@@ -182,7 +212,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<unsigned char>(std::begin(gen.data), std::end(gen.data)));
+ surjectionProofCache.ComputeEntrySurjectionProof(entry, wtxid, vchproof, std::vector<unsigned char>(std::begin(gen.data), std::end(gen.data)), vTags);
if (surjectionProofCache.Get(entry, !store)) {
return true;
@@ -199,5 +229,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<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<unsigned char>& 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<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<secp256k1_generator>& vTags)
+{
+ surjectionProofCache.ComputeEntrySurjectionProof(entry, hash, proof, commitment, vTags);
+}
+
// END ELEMENTS
//
### src/script/sigcache.h
@@ -9,6 +9,7 @@
#include <consensus/amount.h>
#include <crypto/sha256.h>
#include <cuckoocache.h>
+#include <hash.h>
#include <script/interpreter.h>
#include <random.h>
#include <span.h>
@@ -43,11 +44,11 @@ static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES +
class SignatureCache
{
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;
+ HashWriter m_salted_hasher_range_proof;
+ HashWriter m_salted_hasher_surjection_proof;
typedef CuckooCache::cache<uint256, SignatureCacheHasher> map_type;
map_type setValid;
std::shared_mutex cs_sigcache;
@@ -56,10 +57,8 @@ class SignatureCache
SignatureCache()
{
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'};
@@ -68,10 +67,8 @@ class SignatureCache
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;
}
SignatureCache(size_t max_size_bytes);
@@ -84,9 +81,13 @@ class SignatureCache
void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const;
// ELEMENTS:
- void ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const;
+ void ComputeEntryRangeProof(uint256& entry,
+ const std::vector<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<unsigned char>& asset_commitment,
+ const CScript& script_pub_key) const;
- void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const;
+ void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<secp256k1_generator>& vTags) const;
bool Get(const uint256& entry, const bool erase);
@@ -145,6 +146,20 @@ class CachingSurjectionProofChecker
[[nodiscard]] bool InitRangeproofCache(size_t max_size_bytes);
[[nodiscard]] bool InitSurjectionproofCache(size_t max_size_bytes);
+// 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<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<unsigned char>& asset_commitment,
+ const CScript& script_pub_key);
+void TestComputeEntrySurjectionProof(uint256& entry,
+ const uint256& hash,
+ const std::vector<unsigned char>& proof,
+ const std::vector<unsigned char>& commitment,
+ const std::vector<secp256k1_generator>& vTags);
+
// END ELEMENTS
//
Risk score
Our methodology →Why this scored 68/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.