From 5c598abbbc1669d1929b2623bf35584916843973 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 1 Oct 2024 17:28:59 +0200 Subject: [PATCH 1/4] fix(dpp): panic when generating more than 12 keys --- .../rs-dpp/src/identity/identity_public_key/random.rs | 10 +++++----- .../src/identity/identity_public_key/v0/random.rs | 7 ++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/rs-dpp/src/identity/identity_public_key/random.rs b/packages/rs-dpp/src/identity/identity_public_key/random.rs index 0dea7a5dee..0cefd293ee 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/random.rs @@ -615,15 +615,15 @@ impl IdentityPublicKey { used_key_matrix[4] = true; //also a master key used_key_matrix[8] = true; //also a master key used_key_matrix[12] = true; //also a master key - main_keys.extend((3..key_count).map(|i| { - Self::random_authentication_key_with_private_key_with_rng( + for i in 3..key_count { + let privkey = Self::random_authentication_key_with_private_key_with_rng( i, rng, Some((i, &mut used_key_matrix)), platform_version, - ) - .unwrap() - })); + )?; + main_keys.push(privkey); + } Ok(main_keys) } diff --git a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs index 13afdd673f..4701b3c969 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs @@ -69,15 +69,16 @@ impl IdentityPublicKeyV0 { used_key_matrix: Option<(KeyCount, &mut UsedKeyMatrix)>, platform_version: &PlatformVersion, ) -> Result<(Self, Vec), ProtocolError> { + const MAX_KEYS: u8 = 16; // we have 16 different permutations possible - let mut binding = [false; 16].to_vec(); + let mut binding = [false; MAX_KEYS as usize].to_vec(); let (key_count, key_matrix) = used_key_matrix.unwrap_or((0, &mut binding)); - if key_count > 16 { + if key_count > MAX_KEYS as u32 { return Err(ProtocolError::PublicKeyGenerationError( "too many keys already created".to_string(), )); } - let key_number = rng.gen_range(0..(12 - key_count as u8)); + let key_number = rng.gen_range(0..(MAX_KEYS - key_count as u8)); // now we need to find the first bool that isn't set to true let mut needed_pos = None; let mut counter = 0; From 345469243678ee23cfef56f1e3b89e6bbd5d391c Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:51:55 +0200 Subject: [PATCH 2/4] fix(dpp): invalid num of keys in used_key_matrix provided --- .../identity/identity_public_key/random.rs | 2 +- .../identity/identity_public_key/v0/random.rs | 38 +++++++++++++++---- .../tests/strategy_tests/main.rs | 1 - 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/rs-dpp/src/identity/identity_public_key/random.rs b/packages/rs-dpp/src/identity/identity_public_key/random.rs index 0cefd293ee..3fddbd5751 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/random.rs @@ -615,7 +615,7 @@ impl IdentityPublicKey { used_key_matrix[4] = true; //also a master key used_key_matrix[8] = true; //also a master key used_key_matrix[12] = true; //also a master key - for i in 3..key_count { + for i in 6..key_count { let privkey = Self::random_authentication_key_with_private_key_with_rng( i, rng, diff --git a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs index 4701b3c969..17da345524 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs @@ -69,19 +69,34 @@ impl IdentityPublicKeyV0 { used_key_matrix: Option<(KeyCount, &mut UsedKeyMatrix)>, platform_version: &PlatformVersion, ) -> Result<(Self, Vec), ProtocolError> { - const MAX_KEYS: u8 = 16; // we have 16 different permutations possible + const MAX_KEYS: KeyCount = 16; + let mut binding = [false; MAX_KEYS as usize].to_vec(); let (key_count, key_matrix) = used_key_matrix.unwrap_or((0, &mut binding)); - if key_count > MAX_KEYS as u32 { + + // input validation + if key_count != key_matrix.iter().filter(|x| **x).count() as u32 { + return Err(ProtocolError::PublicKeyGenerationError( + "invalid argument: key count in used_key_matrix.0 doesn't match actual number of used keys".to_string(), + )); + } + + // we need space for at least one additional key + if key_count > MAX_KEYS - 1 { return Err(ProtocolError::PublicKeyGenerationError( "too many keys already created".to_string(), )); } - let key_number = rng.gen_range(0..(MAX_KEYS - key_count as u8)); - // now we need to find the first bool that isn't set to true + + // max_key_number is the number of keys that can be created + let max_key_number = MAX_KEYS - key_count; + let key_number = rng.gen_range(0..max_key_number); + // now we need to find n'th not used key of this key_matrix (where `n = key_number`), + // that is: the first bool that isn't set to true let mut needed_pos = None; let mut counter = 0; + let mut used = 0; key_matrix.iter_mut().enumerate().for_each(|(pos, is_set)| { if !*is_set { if counter == key_number { @@ -89,11 +104,20 @@ impl IdentityPublicKeyV0 { *is_set = true; } counter += 1; + } else { + used += 1; // for debugging purposes } }); - let needed_pos = needed_pos.ok_or(ProtocolError::PublicKeyGenerationError( - "too many keys already created".to_string(), - ))?; + // should never happen, as we have input validation above + assert_eq!( + used, key_count, + "incorrect number of used keys in key_matrix {:?}", + key_matrix, + ); + let needed_pos = needed_pos.ok_or(ProtocolError::PublicKeyGenerationError(format!( + "too many keys already created: , key_count: {}, random key number: {}, unused key counter: {}, used key counter: {}", + key_count, key_number, counter, used, + )))?; let key_type = needed_pos.div(&4); let security_level = needed_pos.rem(&4); let security_level = SecurityLevel::try_from(security_level).unwrap(); diff --git a/packages/rs-drive-abci/tests/strategy_tests/main.rs b/packages/rs-drive-abci/tests/strategy_tests/main.rs index 98e2b518a2..17a0a35438 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/main.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/main.rs @@ -47,7 +47,6 @@ mod tests { use crate::execution::{continue_chain_for_strategy, run_chain_for_strategy}; use crate::query::QueryStrategy; use crate::strategy::{FailureStrategy, MasternodeListChangesStrategy}; - use assert_matches::assert_matches; use dashcore_rpc::dashcore::hashes::Hash; use dashcore_rpc::dashcore::BlockHash; use dashcore_rpc::json::QuorumType; From e03289d738d97e2f9ebb21452b74b459b8ca91e1 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:05:22 +0200 Subject: [PATCH 3/4] fix(dpp): add input validation --- .../rs-dpp/src/identity/identity_public_key/random.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/rs-dpp/src/identity/identity_public_key/random.rs b/packages/rs-dpp/src/identity/identity_public_key/random.rs index 3fddbd5751..4fed36eb0f 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/random.rs @@ -583,6 +583,14 @@ impl IdentityPublicKey { "at least 2 keys must be created".to_string(), )); } + + if key_count > 16 { + return Err(ProtocolError::PublicKeyGenerationError(format!( + "too many keys requested: {}, max is 16", + key_count + ))); + } + //create a master and a high level key let mut main_keys = if key_count == 2 { vec![ From 0879ca088e3c1d865f241d3db2ed6b62e6dd40a3 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:45:22 +0200 Subject: [PATCH 4/4] fix: use correct key id --- .../identity/identity_public_key/random.rs | 20 ++++++++++++------- .../identity/identity_public_key/v0/random.rs | 8 ++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/rs-dpp/src/identity/identity_public_key/random.rs b/packages/rs-dpp/src/identity/identity_public_key/random.rs index 4fed36eb0f..b43155e056 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/random.rs @@ -9,6 +9,7 @@ use rand::rngs::StdRng; use rand::SeedableRng; pub type UsedKeyMatrix = Vec; +pub const MAX_RANDOM_KEYS: usize = 16; impl IdentityPublicKey { pub fn random_key(id: KeyID, seed: Option, platform_version: &PlatformVersion) -> Self { @@ -542,7 +543,7 @@ impl IdentityPublicKey { rng: &mut StdRng, platform_version: &PlatformVersion, ) -> Result, ProtocolError> { - let mut used_key_matrix = [false; 16].to_vec(); + let mut used_key_matrix = [false; MAX_RANDOM_KEYS].to_vec(); (0..key_count) .map(|i| { Self::random_authentication_key_with_rng( @@ -584,10 +585,10 @@ impl IdentityPublicKey { )); } - if key_count > 16 { + if key_count > MAX_RANDOM_KEYS as u32 { return Err(ProtocolError::PublicKeyGenerationError(format!( - "too many keys requested: {}, max is 16", - key_count + "too many keys requested: {}, max is {}", + key_count, MAX_RANDOM_KEYS ))); } @@ -616,18 +617,23 @@ impl IdentityPublicKey { )?, ] }; - let mut used_key_matrix = [false; 16].to_vec(); + let mut used_key_matrix = [false; MAX_RANDOM_KEYS].to_vec(); used_key_matrix[0] = true; used_key_matrix[1] = true; used_key_matrix[2] = true; used_key_matrix[4] = true; //also a master key used_key_matrix[8] = true; //also a master key used_key_matrix[12] = true; //also a master key - for i in 6..key_count { + let used_key_matrix_count = used_key_matrix.iter().filter(|x| **x).count() as u32; + let main_keys_count = main_keys.len() as u32; + for i in main_keys_count..key_count { let privkey = Self::random_authentication_key_with_private_key_with_rng( i, rng, - Some((i, &mut used_key_matrix)), + Some(( + used_key_matrix_count + (i - main_keys_count), + &mut used_key_matrix, + )), platform_version, )?; main_keys.push(privkey); diff --git a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs index 17da345524..8bb6d7751f 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/v0/random.rs @@ -1,4 +1,5 @@ use crate::identity::contract_bounds::ContractBounds; +use crate::identity::identity_public_key::random::MAX_RANDOM_KEYS; use crate::identity::identity_public_key::v0::IdentityPublicKeyV0; use crate::identity::KeyType::{ECDSA_HASH160, ECDSA_SECP256K1}; use crate::identity::Purpose::{AUTHENTICATION, VOTING}; @@ -70,9 +71,8 @@ impl IdentityPublicKeyV0 { platform_version: &PlatformVersion, ) -> Result<(Self, Vec), ProtocolError> { // we have 16 different permutations possible - const MAX_KEYS: KeyCount = 16; - let mut binding = [false; MAX_KEYS as usize].to_vec(); + let mut binding = [false; MAX_RANDOM_KEYS].to_vec(); let (key_count, key_matrix) = used_key_matrix.unwrap_or((0, &mut binding)); // input validation @@ -83,14 +83,14 @@ impl IdentityPublicKeyV0 { } // we need space for at least one additional key - if key_count > MAX_KEYS - 1 { + if key_count > MAX_RANDOM_KEYS as u32 - 1 { return Err(ProtocolError::PublicKeyGenerationError( "too many keys already created".to_string(), )); } // max_key_number is the number of keys that can be created - let max_key_number = MAX_KEYS - key_count; + let max_key_number = MAX_RANDOM_KEYS as u32 - key_count; let key_number = rng.gen_range(0..max_key_number); // now we need to find n'th not used key of this key_matrix (where `n = key_number`), // that is: the first bool that isn't set to true