From 609054eae0c7c8cf08645cc6399fd11e840a6d96 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 15 Aug 2018 15:43:29 -0400 Subject: [PATCH] Handle duplicate funding transaction gracefully. This can otherwise cause strange behavior, including a panic on force_close_all_channels found by full_stack_target fuzzing. --- fuzz/fuzz_targets/full_stack_target.rs | 14 +++++++++++--- src/ln/channelmanager.rs | 23 +++++++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs index 8f7ec2df0..54cb838f8 100644 --- a/fuzz/fuzz_targets/full_stack_target.rs +++ b/fuzz/fuzz_targets/full_stack_target.rs @@ -300,13 +300,21 @@ pub fn do_test(data: &[u8]) { } }, 10 => { - for funding_generation in pending_funding_generation.drain(..) { + for funding_generation in pending_funding_generation.drain(..) { let mut tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: vec![TxOut { value: funding_generation.1, script_pubkey: funding_generation.2, }] }; let funding_output = OutPoint::new(Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), 0); - channelmanager.funding_transaction_generated(&funding_generation.0, funding_output.clone()); - pending_funding_signatures.insert(funding_output, tx); + let mut found_duplicate_txo = false; + for chan in channelmanager.list_channels() { + if chan.channel_id == funding_output.to_channel_id() { + found_duplicate_txo = true; + } + } + if !found_duplicate_txo { + channelmanager.funding_transaction_generated(&funding_generation.0, funding_output.clone()); + pending_funding_signatures.insert(funding_output, tx); + } } }, 11 => { diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index 14b21e23a..e142e8ad5 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -770,6 +770,8 @@ impl ChannelManager { /// Call this upon creation of a funding transaction for the given channel. /// Panics if a funding transaction has already been provided for this channel. + /// May panic if the funding_txo is duplicative with some other channel (note that this should + /// be trivially prevented by using unique funding transaction keys per-channel). pub fn funding_transaction_generated(&self, temporary_channel_id: &[u8; 32], funding_txo: OutPoint) { macro_rules! add_pending_event { @@ -812,7 +814,14 @@ impl ChannelManager { }); let mut channel_state = self.channel_state.lock().unwrap(); - channel_state.by_id.insert(chan.channel_id(), chan); + match channel_state.by_id.entry(chan.channel_id()) { + hash_map::Entry::Occupied(_) => { + panic!("Generated duplicate funding txid?"); + }, + hash_map::Entry::Vacant(e) => { + e.insert(chan); + } + } } fn get_announcement_sigs(&self, chan: &Channel) -> Result, HandleError> { @@ -1350,7 +1359,17 @@ impl ChannelMessageHandler for ChannelManager { unimplemented!(); } let mut channel_state = self.channel_state.lock().unwrap(); - channel_state.by_id.insert(funding_msg.channel_id, chan); + match channel_state.by_id.entry(funding_msg.channel_id) { + hash_map::Entry::Occupied(_) => { + return Err(HandleError { + err: "Duplicate channel_id!", + action: Some(msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id: funding_msg.channel_id, data: "Already had channel with the new channel_id".to_owned() } }) + }); + }, + hash_map::Entry::Vacant(e) => { + e.insert(chan); + } + } Ok(funding_msg) } -- 2.39.5