Added missing checks and handling of invalid non-inclusion proof in `…

…smt.rs` (#133) * 🐛 Added missing array length check in inclusion and non-inclusion proof functions * ⬆️ RocksDB updated from 0.19 to 0.21 * 🐛 Fix handling invalid non-inclusion proof
LiskArchive · Sep 20, 2023 · 4c5ff1a · 4c5ff1a
1 parent cc68d1e
commit 4c5ff1a
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 18 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -28,7 +28,7 @@ default-features = false
 features = ["napi-6", "event-queue-api", "try-catch-api"]
 
 [dependencies.rocksdb]
-version = "0.19"
+version = "0.21.0"
 
 [dependencies.hex]
 version = "0.4.3"

diff --git a/sparse_merkle_tree.js b/sparse_merkle_tree.js
@@ -81,6 +81,9 @@ class SparseMerkleTree {
     }
 
     async verifyInclusionProof(root, queries, proof) {
+        if (queries.length !== proof.queries.length) {
+            return false;
+        }
         for (let i = 0; i < queries.length; i++) {
             if (!isInclusionProofForQueryKey(queries[i], proof.queries[i])) {
                 return false;
@@ -90,6 +93,9 @@ class SparseMerkleTree {
     }
 
     async verifyNonInclusionProof(root, queries, proof) {
+        if (queries.length !== proof.queries.length) {
+            return false;
+        }
         for (let i = 0; i < queries.length; i++) {
             if (isInclusionProofForQueryKey(queries[i], proof.queries[i])) {
                 return false;

diff --git a/src/sparse_merkle_tree/smt.rs b/src/sparse_merkle_tree/smt.rs
@@ -873,6 +873,12 @@ impl SparseMerkleTree {
             let binary_bitmap = utils::strip_left_false(&utils::bytes_to_bools(&query.bitmap));
             let query_key_binary = utils::bytes_to_bools(query.key());
             if !utils::is_bytes_equal(key, query.key()) {
+                if query.value().is_empty() {
+                    return Err(SMTError::InvalidInput(String::from(
+                        "Proof for which query key and proof key differs, must have a non-empty value",
+                    )));
+                }
+
                 let key_binary = utils::bytes_to_bools(key);
                 let common_prefix = utils::common_prefix(&key_binary, &query_key_binary);
                 if binary_bitmap.len() > common_prefix.len() {
@@ -3690,7 +3696,21 @@ mod tests {
                     .unwrap_err(),
                 SMTError::InvalidBitmapLen
             );
+            // empty proof value when query key and proof key differs (invalid proof)
+            proof.queries[0] = valid_query_proof.clone();
+            proof.queries[0].pair = Arc::new(KVPair::new(
+                &hex::decode(keys[1]).unwrap(),
+                &hex::decode(vec![]).unwrap(),
+            ));
+            assert_eq!(
+                SparseMerkleTree::verify_and_prepare_proof_map(&proof, &query_keys, KeyLength(32))
+                    .unwrap_err(),
+                SMTError::InvalidInput(String::from(
+                    "Proof for which query key and proof key differs, must have a non-empty value",
+                ))
+            );
             // mismatched binary_bitmap length with query_key_binary length
+            proof.queries[0] = valid_query_proof.clone();
             proof.queries[0].bitmap = Arc::new(vec![30; 33]);
             assert_eq!(
                 SparseMerkleTree::verify_and_prepare_proof_map(&proof, &query_keys, KeyLength(32))

diff --git a/test/sparse_merkle_tree.spec.js b/test/sparse_merkle_tree.spec.js
@@ -46,6 +46,76 @@ describe('SparseMerkleTree', () => {
 		}
 	});
 
+	describe('inclusion and non-inclusion proof', () => {
+		for (const test of [...FixturesInclusionProof.testCases, ...FixturesNonInclusionProof.testCases]) {
+			it(test.description, async () => {
+				const smt = new SparseMerkleTree(32);
+				const inputKeys = test.input.keys;
+				const inputValues = test.input.values;
+				const deletedKeys = test.input.deleteKeys;
+				const queryKeys = test.input.queryKeys.map(keyHex => Buffer.from(keyHex, 'hex'));
+				const outputMerkleRoot = test.output.merkleRoot;
+
+				const kvpair = [];
+				for (let i = 0; i < inputKeys.length; i += 1) {
+					kvpair.push({ key: Buffer.from(inputKeys[i], 'hex'), value: Buffer.from(inputValues[i], 'hex') });
+				}
+
+				for (let i = 0; i < deletedKeys.length; i += 1) {
+					kvpair.push({ key: Buffer.from(deletedKeys[i], 'hex'), value: Buffer.alloc(0) });
+				}
+
+				const rootHash = await smt.update(Buffer.alloc(0), kvpair);
+				expect(rootHash.toString('hex')).toEqual(outputMerkleRoot);
+
+				const proof = await smt.prove(rootHash, queryKeys);
+
+				const isNonInclusionProof = (queriesKeys, proofQueries) => {
+					for (let i = 0; i < queriesKeys.length; i++) {
+						if (isInclusionProofForQueryKey(queriesKeys[i], proofQueries[i])) {
+							return false;
+						}
+					}
+					return true;
+				}
+				const isInclusionProof = (queriesKeys, proofQueries) => {
+					for (let i = 0; i < queriesKeys.length; i++) {
+						if (!isInclusionProofForQueryKey(queriesKeys[i], proofQueries[i])) {
+							return false;
+						}
+					}
+					return true;
+				}
+
+				if (isInclusionProof(queryKeys, proof.queries)) {
+					await expect(smt.verifyInclusionProof(rootHash, queryKeys, proof)).resolves.toEqual(true);
+
+					// modified queryKeys array length should fail
+					const falseQueryKeys = [...queryKeys, getRandomBytes(32)];
+					await expect(smt.verifyInclusionProof(rootHash, falseQueryKeys, proof)).resolves.toEqual(false);
+
+					// modified proof.queries array length should fail
+					const falseProof = { ...proof };
+					falseProof.queries = [...falseProof.queries, falseProof.queries[0]];
+					await expect(smt.verifyInclusionProof(rootHash, queryKeys, falseProof)).resolves.toEqual(false);
+				} else if (isNonInclusionProof(queryKeys, proof.queries)) {
+					await expect(smt.verifyNonInclusionProof(rootHash, queryKeys, proof)).resolves.toEqual(true);
+
+					// modified queryKeys array length should fail
+					const falseQueryKeys = [...queryKeys, getRandomBytes(32)];
+					await expect(smt.verifyNonInclusionProof(rootHash, falseQueryKeys, proof)).resolves.toEqual(false);
+
+					// modified proof.queries array length should fail
+					const falseProof = { ...proof };
+					falseProof.queries = [...falseProof.queries, falseProof.queries[0]];
+					await expect(smt.verifyNonInclusionProof(rootHash, queryKeys, falseProof)).resolves.toEqual(false);
+				} else {
+					throw new Error('Invalid test case');
+				}
+			});
+		}
+	});
+
     describe('remove keys from proof', () => {
         for (const test of FixturesInclusionProof.testCases) {
             it(test.description, async () => {