diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp index 30a34f4b23..8f6de45927 100644 --- a/src/script/sigcache.cpp +++ b/src/script/sigcache.cpp @@ -57,7 +57,15 @@ void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Sp // ELEMENTS: void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector& proof, const std::vector& commitment, const std::vector& 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()); + // Commit to field lengths first: without them, distinct argument tuples that + // concatenate to the same byte stream collide to one cache key. + const uint64_t lengths[4] = {proof.size(), commitment.size(), asset_commitment.size(), scriptPubKey.size()}; + hasher.Write(reinterpret_cast(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 SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector& proof, const std::vector& commitment) const { CSHA256 hasher = m_salted_hasher_surjection_proof; diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp index 1023338f29..04eb5203e2 100644 --- a/src/test/blind_tests.cpp +++ b/src/test/blind_tests.cpp @@ -444,4 +444,63 @@ 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. + // proof and scriptPubKey are variable-length fields at opposite ends of the + // key stream. Without field lengths, their boundary can be shifted while + // leaving the bytes fed to SHA-256 unchanged. + 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 VC = conf_value.vchCommitment; + const std::vector AC = conf_asset.vchCommitment; + BOOST_REQUIRE_EQUAL(VC.size(), 33U); + BOOST_REQUIRE_EQUAL(AC.size(), 33U); + + // Primer script: 6a 43 || C1(33) || X(33) || 6a (69 bytes total). + std::vector C1(33, 0x11); C1[0] = 0x12; + std::vector X(33, 0x13); X[0] = 0x14; + std::vector 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 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); + + // Prime the cache with a genuinely valid proof. + BOOST_CHECK(CachingRangeProofChecker(true).VerifyRangeProof(P0, VC, AC, S0, ctx)); + + // Re-split the identical byte stream: the proof absorbs VC, AC and 6a43, + // while the script shrinks to one byte. The proof is invalid for S1. + std::vector 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& VC2 = C1; + const std::vector& AC2 = X; + std::vector s1{0x6a}; + const CScript S1(s1.begin(), s1.end()); + + // With field lengths this is a cache miss and real verification rejects it. + // On the vulnerable implementation the undelimited key collides with P0. + BOOST_CHECK(!CachingRangeProofChecker(true).VerifyRangeProof(P1, VC2, AC2, S1, ctx)); + + secp256k1_context_destroy(ctx); +} BOOST_AUTO_TEST_SUITE_END()