From 721dd2e4132c759a6d946d8a196335e69dcc46ca Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Thu, 14 Nov 2024 13:14:42 -0300 Subject: [PATCH 01/10] Fix possible panic in trie node decode --- substrate/primitives/trie/src/node_codec.rs | 49 +++++++++++++-------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/substrate/primitives/trie/src/node_codec.rs b/substrate/primitives/trie/src/node_codec.rs index 78896988ec4c..16cddc2b037a 100644 --- a/substrate/primitives/trie/src/node_codec.rs +++ b/substrate/primitives/trie/src/node_codec.rs @@ -44,7 +44,7 @@ impl<'a> ByteSliceInput<'a> { fn take(&mut self, count: usize) -> Result, codec::Error> { if self.offset + count > self.data.len() { - return Err("out of data".into()) + return Err("out of data".into()); } let range = self.offset..(self.offset + count); @@ -66,7 +66,7 @@ impl<'a> Input for ByteSliceInput<'a> { fn read_byte(&mut self) -> Result { if self.offset + 1 > self.data.len() { - return Err("out of data".into()) + return Err("out of data".into()); } let byte = self.data[self.offset]; @@ -110,13 +110,17 @@ where NodeHeader::Null => Ok(NodePlan::Empty), NodeHeader::HashedValueBranch(nibble_count) | NodeHeader::Branch(_, nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; + // data should be at least the size of the offset + if data.len() < input.offset { + return Err(Error::BadFormat); + } // check that the padding is valid (if any) if padding && nibble_ops::pad_left(data[input.offset]) != 0 { - return Err(Error::BadFormat) + return Err(Error::BadFormat); } let partial = input.take( - (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) / - nibble_ops::NIBBLE_PER_BYTE, + (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) + / nibble_ops::NIBBLE_PER_BYTE, )?; let partial_padding = nibble_ops::number_padding(nibble_count); let bitmap_range = input.take(BITMAP_LENGTH)?; @@ -154,13 +158,17 @@ where }, NodeHeader::HashedValueLeaf(nibble_count) | NodeHeader::Leaf(nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; + // data should be at least the size of the offset + if data.len() < input.offset { + return Err(Error::BadFormat); + } // check that the padding is valid (if any) if padding && nibble_ops::pad_left(data[input.offset]) != 0 { - return Err(Error::BadFormat) + return Err(Error::BadFormat); } let partial = input.take( - (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) / - nibble_ops::NIBBLE_PER_BYTE, + (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) + / nibble_ops::NIBBLE_PER_BYTE, )?; let partial_padding = nibble_ops::number_padding(nibble_count); let value = if contains_hash { @@ -229,12 +237,15 @@ where ) -> Vec { let contains_hash = matches!(&value, Some(Value::Node(..))); let mut output = match (&value, contains_hash) { - (&None, _) => - partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchNoValue), - (_, false) => - partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchWithValue), - (_, true) => - partial_from_iterator_encode(partial, number_nibble, NodeKind::HashedValueBranch), + (&None, _) => { + partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchNoValue) + }, + (_, false) => { + partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchWithValue) + }, + (_, true) => { + partial_from_iterator_encode(partial, number_nibble, NodeKind::HashedValueBranch) + }, }; let bitmap_index = output.len(); @@ -285,10 +296,12 @@ fn partial_from_iterator_encode>( NodeKind::Leaf => NodeHeader::Leaf(nibble_count).encode_to(&mut output), NodeKind::BranchWithValue => NodeHeader::Branch(true, nibble_count).encode_to(&mut output), NodeKind::BranchNoValue => NodeHeader::Branch(false, nibble_count).encode_to(&mut output), - NodeKind::HashedValueLeaf => - NodeHeader::HashedValueLeaf(nibble_count).encode_to(&mut output), - NodeKind::HashedValueBranch => - NodeHeader::HashedValueBranch(nibble_count).encode_to(&mut output), + NodeKind::HashedValueLeaf => { + NodeHeader::HashedValueLeaf(nibble_count).encode_to(&mut output) + }, + NodeKind::HashedValueBranch => { + NodeHeader::HashedValueBranch(nibble_count).encode_to(&mut output) + }, }; output.extend(partial); output From 34f3d4007a0449f9dbc368e2797256835246280b Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Thu, 14 Nov 2024 13:22:28 -0300 Subject: [PATCH 02/10] Minor bump to `sp-trie` version --- Cargo.lock | 72 ++++++++++++++-------------- substrate/primitives/trie/Cargo.toml | 2 +- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 775a7fe99e1e..2cabe0300d4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2298,7 +2298,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-std 14.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "trie-db", ] @@ -2342,7 +2342,7 @@ dependencies = [ "sp-core 28.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -2885,7 +2885,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-weights 27.0.0", "staging-xcm 7.0.0", "static_assertions", @@ -4529,7 +4529,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "substrate-prometheus-endpoint", "tracing", @@ -4625,7 +4625,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-storage 19.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "tracing", ] @@ -4811,7 +4811,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-std 14.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -5097,7 +5097,7 @@ dependencies = [ "scale-info", "sp-api 26.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "staging-xcm 7.0.0", ] @@ -5128,7 +5128,7 @@ dependencies = [ "scale-info", "sp-core 28.0.0", "sp-inherents 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -5155,7 +5155,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -5185,7 +5185,7 @@ dependencies = [ "scale-info", "sp-io 30.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -5426,7 +5426,7 @@ dependencies = [ "polkadot-primitives 7.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -7017,7 +7017,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-timestamp 26.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "sp-wasm-interface 20.0.0", "substrate-test-runtime", @@ -7336,7 +7336,7 @@ dependencies = [ "sp-std 14.0.0", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-weights 27.0.0", "static_assertions", "tt-call", @@ -11112,7 +11112,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "tempfile", ] @@ -12384,7 +12384,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -15097,7 +15097,7 @@ dependencies = [ "sp-session 27.0.0", "sp-staking 26.0.0", "sp-state-machine 0.35.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -17118,7 +17118,7 @@ dependencies = [ "quickcheck", "reed-solomon-novelpoly", "sp-core 28.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "thiserror", ] @@ -18747,7 +18747,7 @@ dependencies = [ "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", "sp-transaction-storage-proof 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "sp-version-proc-macro 13.0.0", "sp-wasm-interface 20.0.0", @@ -19553,7 +19553,7 @@ dependencies = [ "sp-session 27.0.0", "sp-staking 26.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -21012,7 +21012,7 @@ dependencies = [ "sp-rpc", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "staging-xcm 7.0.0", "thiserror", @@ -21419,7 +21419,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -22049,7 +22049,7 @@ dependencies = [ "sp-inherents 26.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "substrate-test-runtime-client", ] @@ -22162,7 +22162,7 @@ dependencies = [ "sp-statement-store 10.0.0", "sp-storage 19.0.0", "sp-test-primitives", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "substrate-prometheus-endpoint", "substrate-test-runtime", "thiserror", @@ -22196,7 +22196,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "substrate-test-runtime-client", "tempfile", ] @@ -22599,7 +22599,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "sp-wasm-interface 20.0.0", "substrate-test-runtime", @@ -23345,7 +23345,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-transaction-pool 26.0.0", "sp-transaction-storage-proof 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "static_init", "substrate-prometheus-endpoint", @@ -23387,7 +23387,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-storage 19.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "substrate-test-runtime", "substrate-test-runtime-client", "tempfile", @@ -25407,7 +25407,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-test-primitives", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "thiserror", ] @@ -26471,7 +26471,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "tracing", "tracing-core", ] @@ -26834,7 +26834,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-std 14.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-weights 27.0.0", "substrate-test-runtime-client", "tracing", @@ -27183,7 +27183,7 @@ dependencies = [ "sp-externalities 0.25.0", "sp-panic-handler 13.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "thiserror", "tracing", "trie-db", @@ -27477,7 +27477,7 @@ dependencies = [ "sp-core 28.0.0", "sp-inherents 26.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.0", + "sp-trie 29.0.1", ] [[package]] @@ -27497,7 +27497,7 @@ dependencies = [ [[package]] name = "sp-trie" -version = "29.0.0" +version = "29.0.1" dependencies = [ "ahash 0.8.11", "array-bytes", @@ -28401,7 +28401,7 @@ dependencies = [ "sp-consensus-grandpa 13.0.0", "sp-core 28.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "structopt", "strum 0.26.3", "thiserror", @@ -28450,7 +28450,7 @@ dependencies = [ "sp-core 28.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "trie-db", ] @@ -28524,7 +28524,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.0", + "sp-trie 29.0.1", "sp-version 29.0.0", "substrate-test-runtime-client", "substrate-wasm-builder 17.0.0", diff --git a/substrate/primitives/trie/Cargo.toml b/substrate/primitives/trie/Cargo.toml index 7f27bb097290..bf76122acfbc 100644 --- a/substrate/primitives/trie/Cargo.toml +++ b/substrate/primitives/trie/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sp-trie" -version = "29.0.0" +version = "29.0.1" authors.workspace = true description = "Patricia trie stuff using a parity-scale-codec node format" repository.workspace = true From 1e12ba1bcebc5161605fed0739102d6285ff46e2 Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Thu, 14 Nov 2024 13:40:10 -0300 Subject: [PATCH 03/10] fmt with nightly --- substrate/primitives/trie/src/node_codec.rs | 45 +++++++++------------ 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/substrate/primitives/trie/src/node_codec.rs b/substrate/primitives/trie/src/node_codec.rs index 16cddc2b037a..27da0c6334a2 100644 --- a/substrate/primitives/trie/src/node_codec.rs +++ b/substrate/primitives/trie/src/node_codec.rs @@ -44,7 +44,7 @@ impl<'a> ByteSliceInput<'a> { fn take(&mut self, count: usize) -> Result, codec::Error> { if self.offset + count > self.data.len() { - return Err("out of data".into()); + return Err("out of data".into()) } let range = self.offset..(self.offset + count); @@ -66,7 +66,7 @@ impl<'a> Input for ByteSliceInput<'a> { fn read_byte(&mut self) -> Result { if self.offset + 1 > self.data.len() { - return Err("out of data".into()); + return Err("out of data".into()) } let byte = self.data[self.offset]; @@ -112,15 +112,15 @@ where let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; // data should be at least the size of the offset if data.len() < input.offset { - return Err(Error::BadFormat); + return Err(Error::BadFormat) } // check that the padding is valid (if any) if padding && nibble_ops::pad_left(data[input.offset]) != 0 { - return Err(Error::BadFormat); + return Err(Error::BadFormat) } let partial = input.take( - (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) - / nibble_ops::NIBBLE_PER_BYTE, + (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) / + nibble_ops::NIBBLE_PER_BYTE, )?; let partial_padding = nibble_ops::number_padding(nibble_count); let bitmap_range = input.take(BITMAP_LENGTH)?; @@ -160,15 +160,15 @@ where let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; // data should be at least the size of the offset if data.len() < input.offset { - return Err(Error::BadFormat); + return Err(Error::BadFormat) } // check that the padding is valid (if any) if padding && nibble_ops::pad_left(data[input.offset]) != 0 { - return Err(Error::BadFormat); + return Err(Error::BadFormat) } let partial = input.take( - (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) - / nibble_ops::NIBBLE_PER_BYTE, + (nibble_count + (nibble_ops::NIBBLE_PER_BYTE - 1)) / + nibble_ops::NIBBLE_PER_BYTE, )?; let partial_padding = nibble_ops::number_padding(nibble_count); let value = if contains_hash { @@ -237,15 +237,12 @@ where ) -> Vec { let contains_hash = matches!(&value, Some(Value::Node(..))); let mut output = match (&value, contains_hash) { - (&None, _) => { - partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchNoValue) - }, - (_, false) => { - partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchWithValue) - }, - (_, true) => { - partial_from_iterator_encode(partial, number_nibble, NodeKind::HashedValueBranch) - }, + (&None, _) => + partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchNoValue), + (_, false) => + partial_from_iterator_encode(partial, number_nibble, NodeKind::BranchWithValue), + (_, true) => + partial_from_iterator_encode(partial, number_nibble, NodeKind::HashedValueBranch), }; let bitmap_index = output.len(); @@ -296,12 +293,10 @@ fn partial_from_iterator_encode>( NodeKind::Leaf => NodeHeader::Leaf(nibble_count).encode_to(&mut output), NodeKind::BranchWithValue => NodeHeader::Branch(true, nibble_count).encode_to(&mut output), NodeKind::BranchNoValue => NodeHeader::Branch(false, nibble_count).encode_to(&mut output), - NodeKind::HashedValueLeaf => { - NodeHeader::HashedValueLeaf(nibble_count).encode_to(&mut output) - }, - NodeKind::HashedValueBranch => { - NodeHeader::HashedValueBranch(nibble_count).encode_to(&mut output) - }, + NodeKind::HashedValueLeaf => + NodeHeader::HashedValueLeaf(nibble_count).encode_to(&mut output), + NodeKind::HashedValueBranch => + NodeHeader::HashedValueBranch(nibble_count).encode_to(&mut output), }; output.extend(partial); output From 99db4229d3df81f70dc9007e1aa89af1e38624c0 Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Thu, 14 Nov 2024 15:27:57 -0300 Subject: [PATCH 04/10] Add prdoc --- prdoc/pr_6486.prdoc | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 prdoc/pr_6486.prdoc diff --git a/prdoc/pr_6486.prdoc b/prdoc/pr_6486.prdoc new file mode 100644 index 000000000000..e401d3f9a887 --- /dev/null +++ b/prdoc/pr_6486.prdoc @@ -0,0 +1,10 @@ +title: "sp-trie: minor fix to avoid panic on badly-constructed proof" + +doc: + - audience: ["Runtime Dev", "Runtime User"] + description: | + "Added a check when decoding encoded proof nodes in `sp-trie` to avoid panicking when receiving a badly constructed proof, instead erroring out." + +crates: +- name: sp-trie + bump: patch From 59b14755b79ffa062d20039e9337815a13f4c31d Mon Sep 17 00:00:00 2001 From: Tobi Demeco <50408393+TDemeco@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:21:44 -0300 Subject: [PATCH 05/10] Update substrate/primitives/trie/Cargo.toml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- substrate/primitives/trie/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/primitives/trie/Cargo.toml b/substrate/primitives/trie/Cargo.toml index bf76122acfbc..7f27bb097290 100644 --- a/substrate/primitives/trie/Cargo.toml +++ b/substrate/primitives/trie/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sp-trie" -version = "29.0.1" +version = "29.0.0" authors.workspace = true description = "Patricia trie stuff using a parity-scale-codec node format" repository.workspace = true From d37fc9812b00d485f12b9c6746c5405912f7a276 Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Thu, 14 Nov 2024 18:26:43 -0300 Subject: [PATCH 06/10] update Cargo.lock after reverting `sp-trie` version bump --- Cargo.lock | 72 +++++++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2cabe0300d4c..775a7fe99e1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2298,7 +2298,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-std 14.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "trie-db", ] @@ -2342,7 +2342,7 @@ dependencies = [ "sp-core 28.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -2885,7 +2885,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-weights 27.0.0", "staging-xcm 7.0.0", "static_assertions", @@ -4529,7 +4529,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "substrate-prometheus-endpoint", "tracing", @@ -4625,7 +4625,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-storage 19.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "tracing", ] @@ -4811,7 +4811,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-std 14.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -5097,7 +5097,7 @@ dependencies = [ "scale-info", "sp-api 26.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "staging-xcm 7.0.0", ] @@ -5128,7 +5128,7 @@ dependencies = [ "scale-info", "sp-core 28.0.0", "sp-inherents 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -5155,7 +5155,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -5185,7 +5185,7 @@ dependencies = [ "scale-info", "sp-io 30.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -5426,7 +5426,7 @@ dependencies = [ "polkadot-primitives 7.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -7017,7 +7017,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-timestamp 26.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "sp-wasm-interface 20.0.0", "substrate-test-runtime", @@ -7336,7 +7336,7 @@ dependencies = [ "sp-std 14.0.0", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-weights 27.0.0", "static_assertions", "tt-call", @@ -11112,7 +11112,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-timestamp 26.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "tempfile", ] @@ -12384,7 +12384,7 @@ dependencies = [ "sp-io 30.0.0", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -15097,7 +15097,7 @@ dependencies = [ "sp-session 27.0.0", "sp-staking 26.0.0", "sp-state-machine 0.35.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -17118,7 +17118,7 @@ dependencies = [ "quickcheck", "reed-solomon-novelpoly", "sp-core 28.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "thiserror", ] @@ -18747,7 +18747,7 @@ dependencies = [ "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", "sp-transaction-storage-proof 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "sp-version-proc-macro 13.0.0", "sp-wasm-interface 20.0.0", @@ -19553,7 +19553,7 @@ dependencies = [ "sp-session 27.0.0", "sp-staking 26.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -21012,7 +21012,7 @@ dependencies = [ "sp-rpc", "sp-runtime 31.0.1", "sp-std 14.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "staging-xcm 7.0.0", "thiserror", @@ -21419,7 +21419,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "staging-xcm 7.0.0", "staging-xcm-builder 7.0.0", @@ -22049,7 +22049,7 @@ dependencies = [ "sp-inherents 26.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "substrate-test-runtime-client", ] @@ -22162,7 +22162,7 @@ dependencies = [ "sp-statement-store 10.0.0", "sp-storage 19.0.0", "sp-test-primitives", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "substrate-prometheus-endpoint", "substrate-test-runtime", "thiserror", @@ -22196,7 +22196,7 @@ dependencies = [ "sp-runtime 31.0.1", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "substrate-test-runtime-client", "tempfile", ] @@ -22599,7 +22599,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "sp-wasm-interface 20.0.0", "substrate-test-runtime", @@ -23345,7 +23345,7 @@ dependencies = [ "sp-storage 19.0.0", "sp-transaction-pool 26.0.0", "sp-transaction-storage-proof 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "static_init", "substrate-prometheus-endpoint", @@ -23387,7 +23387,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-storage 19.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "substrate-test-runtime", "substrate-test-runtime-client", "tempfile", @@ -25407,7 +25407,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-test-primitives", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "thiserror", ] @@ -26471,7 +26471,7 @@ dependencies = [ "sp-runtime-interface 24.0.0", "sp-state-machine 0.35.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "tracing", "tracing-core", ] @@ -26834,7 +26834,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-std 14.0.0", "sp-tracing 16.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-weights 27.0.0", "substrate-test-runtime-client", "tracing", @@ -27183,7 +27183,7 @@ dependencies = [ "sp-externalities 0.25.0", "sp-panic-handler 13.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "thiserror", "tracing", "trie-db", @@ -27477,7 +27477,7 @@ dependencies = [ "sp-core 28.0.0", "sp-inherents 26.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.1", + "sp-trie 29.0.0", ] [[package]] @@ -27497,7 +27497,7 @@ dependencies = [ [[package]] name = "sp-trie" -version = "29.0.1" +version = "29.0.0" dependencies = [ "ahash 0.8.11", "array-bytes", @@ -28401,7 +28401,7 @@ dependencies = [ "sp-consensus-grandpa 13.0.0", "sp-core 28.0.0", "sp-runtime 31.0.1", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "structopt", "strum 0.26.3", "thiserror", @@ -28450,7 +28450,7 @@ dependencies = [ "sp-core 28.0.0", "sp-runtime 31.0.1", "sp-state-machine 0.35.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "trie-db", ] @@ -28524,7 +28524,7 @@ dependencies = [ "sp-state-machine 0.35.0", "sp-tracing 16.0.0", "sp-transaction-pool 26.0.0", - "sp-trie 29.0.1", + "sp-trie 29.0.0", "sp-version 29.0.0", "substrate-test-runtime-client", "substrate-wasm-builder 17.0.0", From e81eb3173cb631c51667dac126af194b1baa02eb Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Fri, 15 Nov 2024 11:59:31 -0300 Subject: [PATCH 07/10] =?UTF-8?q?fix=20oversight=20of=20zero-based=20index?= =?UTF-8?q?ing=20(=F0=9F=98=85)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- substrate/primitives/trie/src/node_codec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/trie/src/node_codec.rs b/substrate/primitives/trie/src/node_codec.rs index 27da0c6334a2..603b4ac0f172 100644 --- a/substrate/primitives/trie/src/node_codec.rs +++ b/substrate/primitives/trie/src/node_codec.rs @@ -111,7 +111,7 @@ where NodeHeader::HashedValueBranch(nibble_count) | NodeHeader::Branch(_, nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; // data should be at least the size of the offset - if data.len() < input.offset { + if data.len() <= input.offset { return Err(Error::BadFormat) } // check that the padding is valid (if any) @@ -159,7 +159,7 @@ where NodeHeader::HashedValueLeaf(nibble_count) | NodeHeader::Leaf(nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; // data should be at least the size of the offset - if data.len() < input.offset { + if data.len() <= input.offset { return Err(Error::BadFormat) } // check that the padding is valid (if any) From 4ee66cac4d4ee7d289ce74e85663ca2c653817f5 Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Fri, 15 Nov 2024 12:30:51 -0300 Subject: [PATCH 08/10] refactor to make it more clear + add test --- substrate/primitives/trie/src/node_codec.rs | 8 ++++---- substrate/primitives/trie/src/storage_proof.rs | 10 +++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/substrate/primitives/trie/src/node_codec.rs b/substrate/primitives/trie/src/node_codec.rs index 603b4ac0f172..400f57f3b1bf 100644 --- a/substrate/primitives/trie/src/node_codec.rs +++ b/substrate/primitives/trie/src/node_codec.rs @@ -110,8 +110,8 @@ where NodeHeader::Null => Ok(NodePlan::Empty), NodeHeader::HashedValueBranch(nibble_count) | NodeHeader::Branch(_, nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; - // data should be at least the size of the offset - if data.len() <= input.offset { + // data should be at least of size offset + 1 + if data.len() < input.offset + 1 { return Err(Error::BadFormat) } // check that the padding is valid (if any) @@ -158,8 +158,8 @@ where }, NodeHeader::HashedValueLeaf(nibble_count) | NodeHeader::Leaf(nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; - // data should be at least the size of the offset - if data.len() <= input.offset { + // data should be at least of size offset + 1 + if data.len() < input.offset + 1 { return Err(Error::BadFormat) } // check that the padding is valid (if any) diff --git a/substrate/primitives/trie/src/storage_proof.rs b/substrate/primitives/trie/src/storage_proof.rs index a9f6298742f6..bf0dc72e650b 100644 --- a/substrate/primitives/trie/src/storage_proof.rs +++ b/substrate/primitives/trie/src/storage_proof.rs @@ -232,7 +232,8 @@ pub mod tests { use super::*; use crate::{tests::create_storage_proof, StorageProof}; - type Layout = crate::LayoutV1; + type Hasher = sp_core::Blake2Hasher; + type Layout = crate::LayoutV1; const TEST_DATA: &[(&[u8], &[u8])] = &[(b"key1", &[1; 64]), (b"key2", &[2; 64]), (b"key3", &[3; 64]), (b"key11", &[4; 64])]; @@ -245,4 +246,11 @@ pub mod tests { Err(StorageProofError::DuplicateNodes) )); } + + #[test] + fn invalid_compact_proof_does_not_panic_when_decoding() { + let invalid_proof = CompactProof { encoded_nodes: vec![vec![135]] }; + let result = invalid_proof.to_memory_db::(None); + assert!(result.is_err()); + } } From f83b64521196a36175c5fd617325f566eab3f691 Mon Sep 17 00:00:00 2001 From: Tobi Demeco Date: Fri, 15 Nov 2024 12:50:40 -0300 Subject: [PATCH 09/10] Add prdoc --- prdoc/pr_6502.prdoc | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 prdoc/pr_6502.prdoc diff --git a/prdoc/pr_6502.prdoc b/prdoc/pr_6502.prdoc new file mode 100644 index 000000000000..4a6c94c5298b --- /dev/null +++ b/prdoc/pr_6502.prdoc @@ -0,0 +1,10 @@ +title: "sp-trie: correctly avoid panicking when decoding bad compact proofs" + +doc: + - audience: ["Runtime Dev", "Runtime User"] + description: | + "Fixed the check introduced in [PR #6486](https://github.com/paritytech/polkadot-sdk/pull/6486). Now `sp-trie` correctly avoids panicking when decoding bad compact proofs." + +crates: +- name: sp-trie + bump: patch From e80ea3e98a77ade77899dafa8bc92c6cdbb7f18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 18 Nov 2024 09:01:23 +0100 Subject: [PATCH 10/10] Update prdoc/pr_6502.prdoc --- prdoc/pr_6502.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_6502.prdoc b/prdoc/pr_6502.prdoc index 4a6c94c5298b..3e2467ed5524 100644 --- a/prdoc/pr_6502.prdoc +++ b/prdoc/pr_6502.prdoc @@ -1,7 +1,7 @@ title: "sp-trie: correctly avoid panicking when decoding bad compact proofs" doc: - - audience: ["Runtime Dev", "Runtime User"] + - audience: "Runtime Dev" description: | "Fixed the check introduced in [PR #6486](https://github.com/paritytech/polkadot-sdk/pull/6486). Now `sp-trie` correctly avoids panicking when decoding bad compact proofs."