diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index 5c83903c4..ad3f9e6a8 100644 --- a/grovedb/src/operations/proof/verify.rs +++ b/grovedb/src/operations/proof/verify.rs @@ -5,7 +5,7 @@ use grovedb_merk::{ element::tree_type::ElementTreeTypeExtensions, proofs::{ execute, - query::{PathKey, QueryProofVerify, VerifyOptions}, + query::{PathKey, QueryProofVerify, VerifyOptions, PROOF_VERSION_LATEST}, Decoder, Node, Op, Query, }, tree::{combine_hash, value_hash, NULL_HASH}, @@ -446,7 +446,12 @@ impl GroveDb { }; let (root_hash, merk_result) = level_query - .execute_proof(merk_proof_bytes, *limit_left, internal_query.left_to_right) + .execute_proof( + merk_proof_bytes, + *limit_left, + internal_query.left_to_right, + PROOF_VERSION_LATEST, // V1 proof: strict mode rejects items in value hash nodes + ) .unwrap() .map_err(|e| { Error::InvalidProof( @@ -1389,6 +1394,7 @@ impl GroveDb { &layer_proof.merk_proof, *limit_left, internal_query.left_to_right, + 0, // V0 proofs: allow items in value hash nodes for backwards compatibility ) .unwrap() .map_err(|e| { @@ -1973,7 +1979,12 @@ impl GroveDb { // Execute the proof to verify and get the root hash let (layer_root_hash, result) = key_query - .execute_proof(¤t_layer.merk_proof, None, true) + .execute_proof( + ¤t_layer.merk_proof, + None, + true, + 0, // V0 proof: allow items in value hash nodes + ) .unwrap() .map_err(|e| { Error::InvalidProof( diff --git a/grovedb/src/tests/sum_tree_tests.rs b/grovedb/src/tests/sum_tree_tests.rs index 3658acf4e..9c4d1971a 100644 --- a/grovedb/src/tests/sum_tree_tests.rs +++ b/grovedb/src/tests/sum_tree_tests.rs @@ -1863,19 +1863,15 @@ mod tests { ); // This hardcoded proof was generated by an older version of the code that - // used KVValueHash nodes for item elements. The new proof format correctly - // uses KV nodes for items (verifier-computed hash) and rejects KVValueHash - // for items to prevent KV→KVValueHash proof forgery attacks. + // used KVValueHash nodes for item elements. V0 proofs allow this for + // backwards compatibility (the KV→KVValueHash forgery check is only + // enforced for V1 proofs). let result = GroveDb::verify_subset_query_get_parent_tree_info(&proof, &path_query, grove_version); assert!( - result.is_err(), - "old-format proof with KVValueHash for items should be rejected" - ); - let err = result.unwrap_err().to_string(); - assert!( - err.contains("must not contain an item element"), - "expected item rejection, got: {err}" + result.is_ok(), + "V0 proof with KVValueHash for items should be accepted for backwards compatibility, got: {:?}", + result.unwrap_err() ); } diff --git a/merk/src/proofs/query/merk_integration_tests.rs b/merk/src/proofs/query/merk_integration_tests.rs index 644c8655e..19d1d611e 100644 --- a/merk/src/proofs/query/merk_integration_tests.rs +++ b/merk/src/proofs/query/merk_integration_tests.rs @@ -26,7 +26,7 @@ use super::{ *, }; use crate::{ - proofs::query::verify, + proofs::query::verify::{self, PROOF_VERSION_LATEST}, test_utils::make_tree_seq, tree::{NoopCommit, PanicSource, RefWalker, TreeNode}, TreeFeatureType::BasicMerkNode, @@ -3821,7 +3821,7 @@ fn test_5_item_tree_tampering_detected_with_elements() { query2.insert_key(vec![1]); let (tampered_root, _tampered_result) = query2 - .execute_proof(tampered.as_slice(), None, true) + .execute_proof(tampered.as_slice(), None, true, PROOF_VERSION_LATEST) .unwrap() .expect("execute_proof failed"); @@ -3994,7 +3994,7 @@ fn test_5_item_tree_tampering_detected_raw_values() { query2.insert_key(vec![1]); let (tampered_root, _tampered_result) = query2 - .execute_proof(tampered.as_slice(), None, true) + .execute_proof(tampered.as_slice(), None, true, PROOF_VERSION_LATEST) .unwrap() .expect("execute_proof failed"); @@ -4123,7 +4123,7 @@ fn test_tampering_detected_invalid_discriminant() { // Use execute_proof to get the computed root let (tampered_root, _tampered_result) = query2 - .execute_proof(tampered.as_slice(), None, true) + .execute_proof(tampered.as_slice(), None, true, PROOF_VERSION_LATEST) .unwrap() .expect("execute_proof failed"); @@ -4241,7 +4241,7 @@ fn test_kvvaluehash_still_used_for_tree_discriminants() { query2.insert_key(key.clone()); } let result = query2 - .execute_proof(tampered.as_slice(), None, true) + .execute_proof(tampered.as_slice(), None, true, PROOF_VERSION_LATEST) .unwrap(); assert!( @@ -4295,7 +4295,9 @@ fn test_execute_proof_rejects_trailing_bytes() { // Append trailing garbage bytes bytes.extend_from_slice(&[0xDE, 0xAD, 0xBE, 0xEF]); - let result = query.execute_proof(bytes.as_slice(), None, true).unwrap(); + let result = query + .execute_proof(bytes.as_slice(), None, true, PROOF_VERSION_LATEST) + .unwrap(); assert!( result.is_err(), diff --git a/merk/src/proofs/query/mod.rs b/merk/src/proofs/query/mod.rs index 434e093ce..0d7b931e7 100644 --- a/merk/src/proofs/query/mod.rs +++ b/merk/src/proofs/query/mod.rs @@ -21,7 +21,7 @@ pub use map::{Map, MapBuilder}; #[cfg(any(feature = "minimal", feature = "verify"))] pub use verify::{ ProofVerificationResult, ProvedKeyOptionalValue, ProvedKeyValue, QueryProofVerify, - VerifyOptions, + VerifyOptions, PROOF_VERSION_LATEST, }; #[cfg(feature = "minimal")] use {super::Op, std::collections::LinkedList}; diff --git a/merk/src/proofs/query/verify.rs b/merk/src/proofs/query/verify.rs index 453d28cde..5de4fb6cc 100644 --- a/merk/src/proofs/query/verify.rs +++ b/merk/src/proofs/query/verify.rs @@ -12,6 +12,14 @@ use crate::{ CryptoHash as MerkHash, CryptoHash, }; +/// The latest proof version. +/// - V0 (0): lenient — permits item elements in KVValueHash nodes +/// (backwards compatibility with older proofs) +/// - V1 (1): strict — rejects item elements in KVValueHash / +/// KVValueHashFeatureType / KVValueHashFeatureTypeWithChildHash nodes +/// to prevent KV-to-KVValueHash proof forgery +pub const PROOF_VERSION_LATEST: u16 = 1; + /// Verify proof against expected hash #[cfg(feature = "minimal")] #[deprecated] @@ -75,11 +83,19 @@ impl Default for VerifyOptions { pub trait QueryProofVerify { /// Verifies the encoded proof with the given query, returning the root /// hash and verification result. + /// + /// `proof_version` controls which security checks are applied: + /// - V0 (0): lenient — permits item elements in KVValueHash nodes + /// (backwards compatibility with older proofs) + /// - V1+ (≥1): strict — rejects item elements in KVValueHash / + /// KVValueHashFeatureType / KVValueHashFeatureTypeWithChildHash nodes + /// to prevent KV-to-KVValueHash proof forgery fn execute_proof( &self, bytes: &[u8], limit: Option, left_to_right: bool, + proof_version: u16, ) -> CostResult<(MerkHash, ProofVerificationResult), Error>; /// Verifies the encoded proof with the given query and expected hash. @@ -109,6 +125,7 @@ impl QueryProofVerify for Query { bytes: &[u8], limit: Option, left_to_right: bool, + proof_version: u16, ) -> CostResult<(MerkHash, ProofVerificationResult), Error> { #[cfg(feature = "proof_debug")] { @@ -338,15 +355,19 @@ impl QueryProofVerify for Query { // elements to prevent KV→KVValueHash forgery where an // attacker substitutes a KV node with KVValueHash to inject // a fake value while keeping the original hash. - let element_type = ElementType::from_serialized_value(value).map_err(|e| { - Error::InvalidProofError(format!( - "cannot determine element type in KVValueHash node: {e}" - )) - })?; - if element_type.has_simple_value_hash() { - return Err(Error::InvalidProofError( - "KVValueHash node must not contain an item element".to_string(), - )); + // Skipped for V0 backwards compatibility. + if proof_version >= 1 { + let element_type = + ElementType::from_serialized_value(value).map_err(|e| { + Error::InvalidProofError(format!( + "cannot determine element type in KVValueHash node: {e}" + )) + })?; + if element_type.has_simple_value_hash() { + return Err(Error::InvalidProofError( + "KVValueHash node must not contain an item element".to_string(), + )); + } } execute_node(key, Some(value), *value_hash, false)?; } @@ -384,16 +405,21 @@ impl QueryProofVerify for Query { println!("Processing KVValueHashFeatureType node"); } // Same check as KVValueHash — reject item elements. - let element_type = ElementType::from_serialized_value(value).map_err(|e| { - Error::InvalidProofError(format!( - "cannot determine element type in KVValueHashFeatureType node: {e}" - )) - })?; - if element_type.has_simple_value_hash() { - return Err(Error::InvalidProofError( - "KVValueHashFeatureType node must not contain an item element" - .to_string(), - )); + // Skipped for V0 backwards compatibility. + if proof_version >= 1 { + let element_type = + ElementType::from_serialized_value(value).map_err(|e| { + Error::InvalidProofError(format!( + "cannot determine element type in KVValueHashFeatureType \ + node: {e}" + )) + })?; + if element_type.has_simple_value_hash() { + return Err(Error::InvalidProofError( + "KVValueHashFeatureType node must not contain an item element" + .to_string(), + )); + } } execute_node(key, Some(value), *value_hash, false)?; } @@ -415,19 +441,23 @@ impl QueryProofVerify for Query { { println!("Processing KVValueHashFeatureTypeWithChildHash node"); } - // Same element-type check as KVValueHashFeatureType - let element_type = ElementType::from_serialized_value(value).map_err(|e| { - Error::InvalidProofError(format!( - "cannot determine element type in \ - KVValueHashFeatureTypeWithChildHash node: {e}" - )) - })?; - if element_type.has_simple_value_hash() { - return Err(Error::InvalidProofError( - "KVValueHashFeatureTypeWithChildHash node must not contain \ - an item element" - .to_string(), - )); + // Same element-type check as KVValueHashFeatureType. + // Skipped for V0 backwards compatibility. + if proof_version >= 1 { + let element_type = + ElementType::from_serialized_value(value).map_err(|e| { + Error::InvalidProofError(format!( + "cannot determine element type in \ + KVValueHashFeatureTypeWithChildHash node: {e}" + )) + })?; + if element_type.has_simple_value_hash() { + return Err(Error::InvalidProofError( + "KVValueHashFeatureTypeWithChildHash node must not contain \ + an item element" + .to_string(), + )); + } } // Verify value integrity: combine_hash(H(value), child_hash) must // equal the provided value_hash. This prevents an attacker from @@ -519,7 +549,7 @@ impl QueryProofVerify for Query { left_to_right: bool, expected_hash: MerkHash, ) -> CostResult { - self.execute_proof(bytes, limit, left_to_right) + self.execute_proof(bytes, limit, left_to_right, PROOF_VERSION_LATEST) .map_ok(|(root_hash, verification_result)| { if root_hash == expected_hash { Ok(verification_result)