diff --git a/docs/algorithms.md b/docs/algorithms.md index 63f1d9a2..5284174f 100644 --- a/docs/algorithms.md +++ b/docs/algorithms.md @@ -29,7 +29,7 @@ node_hash = H(kv_hash, left_child_hash, right_child_hash) Note that the `left_child_hash` and/or `right_child_hash` values may be null since it is possible for the node to have no children or only one child. -In our implementation, the hash function used is Blake2b (chosen for its performance characteristics) but this choice is trivially swappable. +In our implementation, the hash function used is SHA512/256 (SHA512 with output truncated to 256 bits) but this choice is trivially swappable. #### Database Representation diff --git a/src/merk/restore.rs b/src/merk/restore.rs index 8477d885..fb39e982 100644 --- a/src/merk/restore.rs +++ b/src/merk/restore.rs @@ -173,8 +173,11 @@ impl Restorer { 0 }; - // FIXME: this one shouldn't be an assert because it comes from a peer - assert_eq!(self.stated_length, chunks_remaining + 1); + if self.stated_length != chunks_remaining + 1 { + return Err(Error::ChunkProcessing( + "Stated length does not match calculated number of chunks".into(), + )); + } // note that these writes don't happen atomically, which is fine here // because if anything fails during the restore process we will just diff --git a/src/proofs/chunk.rs b/src/proofs/chunk.rs index 16a6d149..df053280 100644 --- a/src/proofs/chunk.rs +++ b/src/proofs/chunk.rs @@ -197,17 +197,20 @@ pub(crate) fn verify_leaf>>( #[cfg(feature = "full")] pub(crate) fn verify_trunk>>(ops: I) -> Result<(ProofTree, usize)> { fn verify_height_proof(tree: &ProofTree) -> Result { - Ok(match tree.child(true) { - Some(child) => { - if let Node::Hash(_) = child.tree.node { - return Err(Error::UnexpectedNode( - "Expected height proof to only contain KV and KVHash nodes".into(), - )); - } - verify_height_proof(&child.tree)? + 1 + let mut height = 1; + let mut cursor = tree; + while let Some(child) = cursor.child(true) { + if let Node::Hash(_) = child.tree.node { + return Err(Error::UnexpectedNode( + "Expected height proof to only contain KV and KVHash + nodes" + .into(), + )); } - None => 1, - }) + height += 1; + cursor = &child.tree; + } + Ok(height) } fn verify_completeness(tree: &ProofTree, remaining_depth: usize, leftmost: bool) -> Result<()> { @@ -253,6 +256,11 @@ pub(crate) fn verify_trunk>>(ops: I) -> Result<(Pr })?; let height = verify_height_proof(&tree)?; + if height > 64 { + // This is a sanity check to prevent stack overflows in `verify_completeness`, + // but any tree above 64 is probably an error (~3.7e19 nodes). + return Err(Error::Tree("Tree is too large".into())); + } let trunk_height = height / 2; if trunk_height < MIN_TRUNK_HEIGHT { @@ -268,12 +276,11 @@ pub(crate) fn verify_trunk>>(ops: I) -> Result<(Pr #[cfg(test)] mod tests { - use std::usize; - use super::super::tree::Tree; use super::*; use crate::test_utils::*; use crate::tree::{NoopCommit, PanicSource, Tree as BaseTree}; + use ed::Encode; #[derive(Default)] struct NodeCounts { @@ -467,4 +474,21 @@ mod tests { assert_eq!(counts.hash, 0); assert_eq!(counts.kvhash, 0); } + + #[test] + #[should_panic(expected = "Tree is too large")] + fn test_verify_height_stack_overflow() { + let height = 5_000u32; + let push_op = |i: u32| Op::Push(Node::KV(i.to_be_bytes().to_vec(), vec![])); + let mut ops = Vec::with_capacity((height * 2) as usize); + ops.push(push_op(0)); + for i in 1..height { + ops.push(push_op(i)); + ops.push(Op::Parent) + } + assert!(ops.encoding_length().unwrap() < 50_000); + println!("Len: {}", ops.encoding_length().unwrap()); + let (_, result_height) = verify_trunk(ops.into_iter().map(Ok)).unwrap(); + assert_eq!(height, result_height as u32); + } } diff --git a/src/proofs/query/mod.rs b/src/proofs/query/mod.rs index 11808910..d0ad7c48 100644 --- a/src/proofs/query/mod.rs +++ b/src/proofs/query/mod.rs @@ -474,6 +474,7 @@ mod test { use super::*; use crate::test_utils::make_tree_seq; use crate::tree::{NoopCommit, PanicSource, RefWalker, Tree}; + use ed::Encode; fn make_3_node_tree() -> Result { let mut tree = Tree::new(vec![5], vec![5])? @@ -1477,4 +1478,18 @@ mod test { let _result = verify_query(bytes.as_slice(), &query, [42; 32]).expect("verify failed"); } + + #[test] + #[should_panic(expected = "Tried to attach to Hash node")] + fn hash_attach() { + let mut target = make_3_node_tree().expect("tree construction failed"); + + let mut proof = Vec::new(); + proof.push(Op::Push(Node::KV(vec![42], vec![42]))); + proof.push(Op::Push(Node::Hash(target.hash()))); + proof.push(Op::Parent); + + let map = verify(&proof.encode().unwrap(), target.hash()).unwrap(); + assert_eq!(map.get(&[42]).unwrap().unwrap(), &[42]) + } } diff --git a/src/proofs/tree.rs b/src/proofs/tree.rs index 68abe74c..b161d105 100644 --- a/src/proofs/tree.rs +++ b/src/proofs/tree.rs @@ -116,8 +116,8 @@ impl Tree { } } - /// Attaches the child to the `Tree`'s given side. Panics if there is - /// already a child attached to this side. + /// Attaches the child to the `Tree`'s given side. Returns an error if + /// there is already a child attached to this side. pub(crate) fn attach(&mut self, left: bool, child: Tree) -> Result<()> { if self.child(left).is_some() { return Err(Error::Attach( @@ -125,6 +125,10 @@ impl Tree { )); } + if let Node::Hash(_) = self.node { + return Err(Error::Attach("Tried to attach to Hash node".into())); + } + self.height = self.height.max(child.height + 1); let hash = child.hash()?; diff --git a/src/tree/hash.rs b/src/tree/hash.rs index e31f2dc8..8c387bef 100644 --- a/src/tree/hash.rs +++ b/src/tree/hash.rs @@ -14,9 +14,6 @@ pub const NULL_HASH: Hash = [0; HASH_LENGTH]; pub type Hash = [u8; HASH_LENGTH]; /// Hashes a key/value pair. -/// -/// **NOTE:** This will fail if the key is longer than 255 bytes, or the value -/// is longer than 65,535 bytes. pub fn kv_hash(key: &[u8], value: &[u8]) -> Result { let mut hasher = D::new(); hasher.update([0]);