From 0b82f5584f31e45ef2c176041c1121181c061bc9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 16 Jul 2018 21:12:54 -0700 Subject: [PATCH] Test channelmonitor serialization roundtrip doesn't mutate state --- fuzz/Cargo.toml | 4 ++ .../channelmonitor_deserialize_target.rs | 63 +++++++++++++++++++ src/ln/chan_utils.rs | 2 +- src/ln/channelmonitor.rs | 40 +++++++++++- src/util/test_utils.rs | 4 ++ 5 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 fuzz/fuzz_targets/channelmonitor_deserialize_target.rs diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index c302292e6..8e2d76ce4 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -42,6 +42,10 @@ path = "fuzz_targets/channel_target.rs" name = "full_stack_target" path = "fuzz_targets/full_stack_target.rs" +[[bin]] +name = "channelmonitor_deserialize_target" +path = "fuzz_targets/channelmonitor_deserialize_target.rs" + # message fuzz targets [[bin]] name = "msg_ping_target" diff --git a/fuzz/fuzz_targets/channelmonitor_deserialize_target.rs b/fuzz/fuzz_targets/channelmonitor_deserialize_target.rs new file mode 100644 index 000000000..0bf86177a --- /dev/null +++ b/fuzz/fuzz_targets/channelmonitor_deserialize_target.rs @@ -0,0 +1,63 @@ +// This file is auto-generated by gen_target.sh based on msg_target_template.txt +// To modify it, modify msg_target_template.txt and run gen_target.sh instead. + +extern crate lightning; + +use lightning::ln::channelmonitor; +use lightning::util::reset_rng_state; + +#[inline] +pub fn do_test(data: &[u8]) { + reset_rng_state(); + if let Some(monitor) = channelmonitor::ChannelMonitor::deserialize(data) { + assert!(channelmonitor::ChannelMonitor::deserialize(&monitor.serialize_for_disk()[..]).unwrap() == monitor); + monitor.serialize_for_watchtower(); + } +} + +#[cfg(feature = "afl")] +extern crate afl; +#[cfg(feature = "afl")] +fn main() { + afl::read_stdio_bytes(|data| { + do_test(&data); + }); +} + +#[cfg(feature = "honggfuzz")] +#[macro_use] extern crate honggfuzz; +#[cfg(feature = "honggfuzz")] +fn main() { + loop { + fuzz!(|data| { + do_test(data); + }); + } +} + +#[cfg(test)] +mod tests { + fn extend_vec_from_hex(hex: &str, out: &mut Vec) { + let mut b = 0; + for (idx, c) in hex.as_bytes().iter().enumerate() { + b <<= 4; + match *c { + b'A'...b'F' => b |= c - b'A' + 10, + b'a'...b'f' => b |= c - b'a' + 10, + b'0'...b'9' => b |= c - b'0', + _ => panic!("Bad hex"), + } + if (idx & 1) == 1 { + out.push(b); + b = 0; + } + } + } + + #[test] + fn duplicate_crash() { + let mut a = Vec::new(); + extend_vec_from_hex("00", &mut a); + super::do_test(&a); + } +} diff --git a/src/ln/chan_utils.rs b/src/ln/chan_utils.rs index eaae62f92..0cace11c1 100644 --- a/src/ln/chan_utils.rs +++ b/src/ln/chan_utils.rs @@ -151,7 +151,7 @@ pub fn get_revokeable_redeemscript(revocation_key: &PublicKey, to_self_delay: u1 .into_script() } -#[derive(Clone)] +#[derive(Clone, PartialEq)] pub struct HTLCOutputInCommitment { pub offered: bool, pub amount_msat: u64, diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 891dac807..d212a3fdb 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -117,7 +117,7 @@ const CLTV_SHARED_CLAIM_BUFFER: u32 = 12; /// HTLC-Success transaction. const CLTV_CLAIM_BUFFER: u32 = 6; -#[derive(Clone)] +#[derive(Clone, PartialEq)] enum KeyStorage { PrivMode { revocation_base_key: SecretKey, @@ -130,7 +130,7 @@ enum KeyStorage { } } -#[derive(Clone)] +#[derive(Clone, PartialEq)] struct LocalSignedTx { /// txid of the transaction in tx, just used to make comparison faster txid: Sha256dHash, @@ -215,6 +215,40 @@ impl Clone for ChannelMonitor { } } +#[cfg(any(test, feature = "fuzztarget"))] +/// Used only in testing and fuzztarget to check serialization roundtrips don't change the +/// underlying object +impl PartialEq for ChannelMonitor { + fn eq(&self, other: &Self) -> bool { + if self.funding_txo != other.funding_txo || + self.commitment_transaction_number_obscure_factor != other.commitment_transaction_number_obscure_factor || + self.key_storage != other.key_storage || + self.delayed_payment_base_key != other.delayed_payment_base_key || + self.their_htlc_base_key != other.their_htlc_base_key || + self.their_cur_revocation_points != other.their_cur_revocation_points || + self.our_to_self_delay != other.our_to_self_delay || + self.their_to_self_delay != other.their_to_self_delay || + self.remote_claimable_outpoints != other.remote_claimable_outpoints || + self.remote_hash_commitment_number != other.remote_hash_commitment_number || + self.prev_local_signed_commitment_tx != other.prev_local_signed_commitment_tx || + self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx || + self.payment_preimages != other.payment_preimages || + self.destination_script != other.destination_script + { + false + } else { + for (&(ref secret, ref idx), &(ref o_secret, ref o_idx)) in self.old_secrets.iter().zip(other.old_secrets.iter()) { + if secret != o_secret || idx != o_idx { + return false + } + } + let us = self.remote_commitment_txn_on_chain.lock().unwrap(); + let them = other.remote_commitment_txn_on_chain.lock().unwrap(); + *us == *them + } + } +} + impl ChannelMonitor { pub fn new(revocation_base_key: &SecretKey, delayed_payment_base_key: &PublicKey, htlc_base_key: &SecretKey, our_to_self_delay: u16, destination_script: Script) -> ChannelMonitor { ChannelMonitor { @@ -603,7 +637,7 @@ impl ChannelMonitor { macro_rules! read_bytes { ($byte_count: expr) => { { - if ($byte_count as usize) + read_pos > data.len() { + if ($byte_count as usize) > data.len() - read_pos { return None; } read_pos += $byte_count as usize; diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs index 658758328..6647020f6 100644 --- a/src/util/test_utils.rs +++ b/src/util/test_utils.rs @@ -30,6 +30,10 @@ impl TestChannelMonitor { } impl channelmonitor::ManyChannelMonitor for TestChannelMonitor { fn add_update_monitor(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor) -> Result<(), channelmonitor::ChannelMonitorUpdateErr> { + // At every point where we get a monitor update, we should be able to send a useful monitor + // to a watchtower and disk... + assert!(channelmonitor::ChannelMonitor::deserialize(&monitor.serialize_for_disk()[..]).unwrap() == monitor); + monitor.serialize_for_watchtower(); // This at least shouldn't crash... self.added_monitors.lock().unwrap().push((funding_txo, monitor.clone())); self.simple_monitor.add_update_monitor(funding_txo, monitor) } -- 2.39.5