Merge pull request #246 from TheBlueMatt/2018-11-fuzz-crash-redux

Several fuzz-found bugfixes.

diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
index ecf3809c59b3ec8cc88ca349b3ad1eff3b9cb7b0..c8ed4b613cf5d795eae7047a4377f0e937d510bb 100644 (file)
--- a/fuzz/Cargo.toml
+++ b/fuzz/Cargo.toml
@@ -31,6 +31,10 @@ cc = "1.0"
 [workspace]
 members = ["."]
 
+[profile.release]
+lto = true
+codegen-units = 1
+
 [[bin]]
 name = "peer_crypt_target"
 path = "fuzz_targets/peer_crypt_target.rs"

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index d547ef3f06734389a6afd9026fcbaa35f09068e5..c87e5a6ef15c6572f75e8035d6c01649b72ba616 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -1988,10 +1988,20 @@ impl ChannelManager {
                                        // but if we've sent a shutdown and they haven't acknowledged it yet, we just
                                        // want to reject the new HTLC and fail it backwards instead of forwarding.
                                        if let PendingHTLCStatus::Forward(PendingForwardHTLCInfo { incoming_shared_secret, .. }) = pending_forward_info {
+                                               let chan_update = self.get_channel_update(chan);
                                                pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
                                                        channel_id: msg.channel_id,
                                                        htlc_id: msg.htlc_id,
-                                                       reason: ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &self.get_channel_update(chan).unwrap().encode_with_len()[..]),
+                                                       reason: if let Ok(update) = chan_update {
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &update.encode_with_len()[..])
+                                                       } else {
+                                                               // This can only happen if the channel isn't in the fully-funded
+                                                               // state yet, implying our counterparty is trying to route payments
+                                                               // over the channel back to themselves (cause no one else should
+                                                               // know the short_id is a lightning channel yet). We should have no
+                                                               // problem just calling this unknown_next_peer
+                                                               ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x4000|10, &[])
+                                                       },
                                                }));
                                        }
                                }

diff --git a/src/ln/peer_channel_encryptor.rs b/src/ln/peer_channel_encryptor.rs
index 6a2033e4532e9069f27f8e3a33c6193e7a5a7daa..eaf2f683279efe83ad1a84c2f37b9c2d9df76dae 100644 (file)
--- a/src/ln/peer_channel_encryptor.rs
+++ b/src/ln/peer_channel_encryptor.rs
@@ -279,7 +279,7 @@ impl PeerChannelEncryptor {
                self.process_act_one_with_ephemeral_key(act_one, our_node_secret, our_ephemeral_key)
        }
 
-       pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<[u8; 66], HandleError> {
+       pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<([u8; 66], PublicKey), HandleError> {
                assert_eq!(act_two.len(), 50);
 
                let mut final_hkdf = [0; 64];
@@ -334,7 +334,7 @@ impl PeerChannelEncryptor {
                        rck: ck,
                };
 
-               Ok(res)
+               Ok((res, self.their_node_id.unwrap().clone()))
        }
 
        pub fn process_act_three(&mut self, act_three: &[u8]) -> Result<PublicKey, HandleError> {
@@ -537,7 +537,7 @@ mod tests {
                        let mut outbound_peer = get_outbound_peer_for_initiator_test_vectors();
 
                        let act_two = hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap().to_vec();
-                       assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap()[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
+                       assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap().0[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
 
                        match outbound_peer.noise_state {
                                NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {
@@ -694,7 +694,7 @@ mod tests {
                        let our_node_id = SecretKey::from_slice(&secp_ctx, &hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap();
 
                        let act_two = hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap().to_vec();
-                       assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap()[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
+                       assert_eq!(outbound_peer.process_act_two(&act_two[..], &our_node_id).unwrap().0[..], hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap()[..]);
 
                        match outbound_peer.noise_state {
                                NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {

diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs
index 87a2e6d1c242b64f6e9b15fb0e9de9ef7cb8f74e..1ea825473a45b86eaf42236475ca206faaabdcf6 100644 (file)
--- a/src/ln/peer_handler.rs
+++ b/src/ln/peer_handler.rs
@@ -234,7 +234,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                if peers.peers.insert(descriptor, Peer {
                        channel_encryptor: peer_encryptor,
                        outbound: true,
-                       their_node_id: Some(their_node_id),
+                       their_node_id: None,
                        their_global_features: None,
                        their_local_features: None,
 
@@ -438,9 +438,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 
                                                        macro_rules! try_potential_handleerror {
                                                                ($thing: expr) => {
-                                                                       try_potential_handleerror!($thing, false);
-                                                               };
-                                                               ($thing: expr, $pre_noise: expr) => {
                                                                        match $thing {
                                                                                Ok(x) => x,
                                                                                Err(e) => {
@@ -449,9 +446,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                                                        msgs::ErrorAction::DisconnectPeer { msg: _ } => {
                                                                                                                //TODO: Try to push msg
                                                                                                                log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
-                                                                                                               if $pre_noise {
-                                                                                                                       peer.their_node_id = None; // Unset so that we don't generate a peer_disconnected event
-                                                                                                               }
                                                                                                                return Err(PeerHandleError{ no_connection_possible: false });
                                                                                                        },
                                                                                                        msgs::ErrorAction::IgnoreError => {
@@ -517,16 +511,17 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                        let next_step = peer.channel_encryptor.get_noise_step();
                                                        match next_step {
                                                                NextNoiseStep::ActOne => {
-                                                                       let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
+                                                                       let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret)).to_vec();
                                                                        peer.pending_outbound_buffer.push_back(act_two);
                                                                        peer.pending_read_buffer = [0; 66].to_vec(); // act three is 66 bytes long
                                                                },
                                                                NextNoiseStep::ActTwo => {
-                                                                       let act_three = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
-                                                                       peer.pending_outbound_buffer.push_back(act_three);
+                                                                       let (act_three, their_node_id) = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret));
+                                                                       peer.pending_outbound_buffer.push_back(act_three.to_vec());
                                                                        peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
                                                                        peer.pending_read_is_header = true;
 
+                                                                       peer.their_node_id = Some(their_node_id);
                                                                        insert_node_id!();
                                                                        let mut local_features = msgs::LocalFeatures::new();
                                                                        if self.initial_syncs_sent.load(Ordering::Acquire) < INITIAL_SYNCS_TO_SEND {
@@ -539,7 +534,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                                        }, 16);
                                                                },
                                                                NextNoiseStep::ActThree => {
-                                                                       let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]), true);
+                                                                       let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]));
                                                                        peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
                                                                        peer.pending_read_is_header = true;
                                                                        peer.their_node_id = Some(their_node_id);