From ac95c5d5139c839b9cf561a025a29ef663ddd25f Mon Sep 17 00:00:00 2001
From: andycreed0x <11888868+andycreed0x@users.noreply.github.com>
Date: Tue, 8 Sep 2026 14:16:04 -0300
Subject: [PATCH] Delimit rangeproof cache key fields to prevent boundary
 collisions

ComputeEntryRangeProof hashed (proof, value commitment, asset commitment,
scriptPubKey) into the cache key by raw concatenation, with no field
delimiters. The two commitments are fixed 33-byte fields, but proof and
scriptPubKey are variable length and sit at opposite ends of the stream,
so the proof/script boundary can be shifted while leaving the concatenated
bytes identical. Two distinct tuples then map to the same key.

Because a positive cache hit returns true without verifying, a node that
had cached one valid rangeproof would accept a different, unverified proof
whose (proof, script) split differs but whose byte stream matches: the
attacker primes the cache with a genuine proof over an OP_RETURN script
carrying padding, then resubmits the same bytes re-split so the padding
counts as proof and the script shrinks to a single byte.

Prepend the four field lengths to the hashed key so the encoding is
injective; no other partition of the same stream yields the same key. The
key is process-local (salted per start, never serialized or compared
across nodes), so the native byte order of the length array is fine.

Add a regression test (blind_tests/rangeproof_cache_key_field_boundary)
that primes the cache with a genuine proof over a 69-byte OP_RETURN script,
then submits an attack tuple that shifts the boundary 68 bytes; the attack
proof is invalid for its one-byte script and must be rejected.
---
 src/script/sigcache.cpp  |  4 ++-
 src/test/blind_tests.cpp | 76 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index 9f7bb9592b..a5c9198239 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -74,7 +74,9 @@ class CSignatureCache
     // 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) {
         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());
+        // Commit to field lengths first: without them, distinct (proof, commitment, asset_commitment, scriptPubKey) tuples that concatenate to the same byte stream collide to one cache key. The key is process-local (salted per start, never serialized or compared across nodes), so the array's native byte order is fine.
+        const uint64_t lengths[4] = {proof.size(), commitment.size(), asset_commitment.size(), scriptPubKey.size()};
+        hasher.Write(reinterpret_cast<const unsigned char*>(lengths), sizeof(lengths)).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 ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) {
         CSHA256 hasher = m_salted_hasher_surjection_proof;
diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp
index 8cb328f195..eae42e5d49 100644
--- a/src/test/blind_tests.cpp
+++ b/src/test/blind_tests.cpp
@@ -6,6 +6,7 @@
 #include <blind.h>
 #include <blindpsbt.h>
 #include <coins.h>
+#include <script/sigcache.h>
 #include <uint256.h>
 #include <validation.h>
 
@@ -440,4 +441,79 @@ BOOST_AUTO_TEST_CASE(rangeproof_zero_value_spendable_script)
     BOOST_CHECK(GenerateRangeproof(rangeproof, value_blindptrs, nonce, 1, spendable,
                                    value_commit_one, asset_gen, asset, asset_blindptrs));
 }
+BOOST_AUTO_TEST_CASE(rangeproof_cache_key_field_boundary)
+{
+    // Regression test for the rangeproof cache-key ambiguity (Bug B).
+    //
+    // ComputeEntryRangeProof hashes (proof || value_commitment ||
+    // asset_commitment || scriptPubKey) into the cache key. The two commitments
+    // are 33 bytes each, but proof and scriptPubKey are variable length and sit
+    // at opposite ends of the stream, so the proof/script boundary can be shifted
+    // without changing the concatenated bytes. Two different tuples then hash to
+    // the same key. Since a cache hit returns true WITHOUT verifying, a node that
+    // has cached one valid proof would accept a bogus tuple whose byte stream is
+    // identical but whose (proof, script) split differs.
+    //
+    // The "primer" is a genuine rangeproof over a 69-byte OP_RETURN script
+    // 6a 43 || C1(33) || X(33) || 6a. The "attack" tuple shifts the boundary 68
+    // bytes: its proof is (primer_proof || value_commit || asset_commit || 6a 43)
+    // and its script is the single trailing 6a. That attack proof is not valid
+    // for the 1-byte script, so it must be rejected. Before the fix it collides
+    // with the primer's cache entry and is wrongly accepted.
+
+    const CAsset asset(GetRandHash());
+    const uint256 asset_blinder = GetRandHash();
+    const uint256 value_blinder = GetRandHash();
+    const uint256 nonce = GetRandHash();
+
+    CConfidentialAsset conf_asset;
+    secp256k1_generator asset_gen;
+    CreateAssetCommitment(conf_asset, asset_gen, asset, asset_blinder);
+
+    CConfidentialValue conf_value;
+    secp256k1_pedersen_commitment value_commit;
+    CreateValueCommitment(conf_value, value_commit, value_blinder.begin(), asset_gen, 1000);
+
+    const std::vector<unsigned char> VC = conf_value.vchCommitment; // 33 bytes
+    const std::vector<unsigned char> AC = conf_asset.vchCommitment; // 33 bytes
+    BOOST_REQUIRE_EQUAL(VC.size(), 33U);
+    BOOST_REQUIRE_EQUAL(AC.size(), 33U);
+
+    // Primer script: 6a 43 || C1(33) || X(33) || 6a (unspendable OP_RETURN).
+    std::vector<unsigned char> C1(33, 0x11); C1[0] = 0x12;
+    std::vector<unsigned char> X(33, 0x13); X[0] = 0x14;
+    std::vector<unsigned char> s0{0x6a, 0x43};
+    s0.insert(s0.end(), C1.begin(), C1.end());
+    s0.insert(s0.end(), X.begin(), X.end());
+    s0.push_back(0x6a);
+    const CScript S0(s0.begin(), s0.end());
+    BOOST_REQUIRE(S0.IsUnspendable());
+
+    std::vector<unsigned char> P0;
+    BOOST_REQUIRE(CreateValueRangeProof(P0, value_blinder, nonce, 1000, S0, value_commit, asset_gen, asset, asset_blinder));
+
+    secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
+
+    // Primer: a genuine proof, stored into the cache (store = true).
+    BOOST_CHECK(CachingRangeProofChecker(true).VerifyRangeProof(P0, VC, AC, S0, ctx));
+
+    // Attack: shift the proof/script boundary 68 bytes right. The concatenation
+    // (P1 || C1 || X || 6a) is byte-for-byte identical to (P0 || VC || AC || S0),
+    // so the un-delimited key collides with the primer's entry.
+    std::vector<unsigned char> P1 = P0;
+    P1.insert(P1.end(), VC.begin(), VC.end());
+    P1.insert(P1.end(), AC.begin(), AC.end());
+    P1.push_back(0x6a); P1.push_back(0x43);
+    const std::vector<unsigned char>& VC2 = C1;
+    const std::vector<unsigned char>& AC2 = X;
+    std::vector<unsigned char> s1{0x6a};
+    const CScript S1(s1.begin(), s1.end());
+
+    // The attack proof is not a valid rangeproof for S1. With the fix it is a
+    // cache miss and real verification rejects it; before the fix it is a cache
+    // hit and wrongly accepted. This assertion fails on the unfixed code.
+    BOOST_CHECK(!CachingRangeProofChecker(true).VerifyRangeProof(P1, VC2, AC2, S1, ctx));
+
+    secp256k1_context_destroy(ctx);
+}
 BOOST_AUTO_TEST_SUITE_END()