From e2a4a5884b444a6ef3f137ef2cc21a816c386acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 29 Sep 2022 14:24:28 +0800 Subject: [PATCH] Ensure backward compatibility of the "checksum inception" bug `Wallet` stores the descriptors' checksum in the database for safety. Previously, the checksum used was a checksum of a descriptor that already had a checksum. This PR allows for backward-compatibility of databases created with this bug. --- src/descriptor/checksum.rs | 2 +- src/descriptor/mod.rs | 2 +- src/wallet/mod.rs | 56 +++++++++++++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 8dfdac49..9eedb45b 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -83,7 +83,7 @@ pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8], // if input data already had a checksum, check calculated checksum against original checksum if let Some(original_checksum) = original_checksum { - if original_checksum.as_bytes() != &checksum { + if original_checksum.as_bytes() != checksum { return Err(DescriptorError::InvalidDescriptorChecksum); } } diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 7c51d27f..4955f8ff 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -88,7 +88,7 @@ impl IntoWalletDescriptor for &str { let descriptor = match self.split_once('#') { Some((desc, original_checksum)) => { let checksum = get_checksum_bytes(desc, false)?; - if original_checksum.as_bytes() != &checksum { + if original_checksum.as_bytes() != checksum { return Err(DescriptorError::InvalidDescriptorChecksum); } desc diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 776e1740..737afe8d 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -64,6 +64,7 @@ use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx}; use crate::blockchain::{GetHeight, NoopProgress, Progress, WalletSync}; use crate::database::memory::MemoryDatabase; use crate::database::{AnyDatabase, BatchDatabase, BatchOperations, DatabaseUtils, SyncTime}; +use crate::descriptor::checksum::get_checksum_bytes; use crate::descriptor::derived::AsDerived; use crate::descriptor::policy::BuildSatisfaction; use crate::descriptor::{ @@ -203,18 +204,21 @@ where let secp = Secp256k1::new(); let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &descriptor.to_string(), KeychainKind::External, - get_checksum(&descriptor.to_string())?.as_bytes(), )?; let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); + let (change_descriptor, change_signers) = match change_descriptor { Some(desc) => { let (change_descriptor, change_keymap) = into_wallet_descriptor_checked(desc, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &change_descriptor.to_string(), KeychainKind::Internal, - get_checksum(&change_descriptor.to_string())?.as_bytes(), )?; let change_signers = Arc::new(SignersContainer::build( @@ -222,9 +226,6 @@ where &change_descriptor, &secp, )); - // if !parsed.same_structure(descriptor.as_ref()) { - // return Err(Error::DifferentDescriptorStructure); - // } (Some(change_descriptor), change_signers) } @@ -243,6 +244,19 @@ where }) } + /// This checks the checksum within [`BatchDatabase`] twice (if needed). The first time with the + /// actual checksum, and the second time with the checksum of `descriptor+checksum`. The second + /// check is necessary for backwards compatibility of a checksum-inception bug. + fn db_checksum(db: &mut D, desc: &str, kind: KeychainKind) -> Result<(), Error> { + let checksum = get_checksum_bytes(desc, true)?; + if db.check_descriptor_checksum(kind, checksum).is_ok() { + return Ok(()); + } + + let checksum_inception = get_checksum_bytes(desc, false)?; + db.check_descriptor_checksum(kind, checksum_inception) + } + /// Get the Bitcoin network the wallet is using. pub fn network(&self) -> Network { self.network @@ -1949,6 +1963,34 @@ pub(crate) mod test { ); } + #[test] + fn test_db_checksum() { + let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); + let desc = wallet.descriptor.to_string(); + + let checksum = get_checksum_bytes(&desc, true).unwrap(); + let checksum_inception = get_checksum_bytes(&desc, false).unwrap(); + let checksum_invalid = [b'q'; 8]; + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum) + .expect("failed to save actual checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses actual checksum should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_inception) + .expect("failed to save checksum inception"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses checksum inception should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_invalid) + .expect("failed to save invalid checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect_err("db that uses invalid checksum should fail"); + } + #[test] fn test_get_funded_wallet_balance() { let (wallet, _, _) = get_funded_wallet(get_test_wpkh());