Merge pull request #2974 from benthecarman/dang-value

Add DecodeError::DangerousValue for decoding invalid channel managers

diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs
index bf94c910531573335f2db2594ce1df41304b0f3d..ad4373c4793bc704f66d6cc095b83a5e82013cf7 100644 (file)
--- a/fuzz/src/router.rs
+++ b/fuzz/src/router.rs
@@ -157,6 +157,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                        msgs::DecodeError::ShortRead => panic!("We picked the length..."),
                                        msgs::DecodeError::Io(e) => panic!("{:?}", e),
                                        msgs::DecodeError::UnsupportedCompression => return,
+                                       msgs::DecodeError::DangerousValue => return,
                                }
                        }
                }}

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index bddcd4c9b8117b7884228161757028d118dc7af0..a15c1beea0bbc1ee5ae0416694cbc3b8c82dbdc0 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -10927,7 +10927,7 @@ where
                                                }
                                        }
                                        if chan.get_latest_unblocked_monitor_update_id() > max_in_flight_update_id {
-                                               // If the channel is ahead of the monitor, return InvalidValue:
+                                               // If the channel is ahead of the monitor, return DangerousValue:
                                                log_error!(logger, "A ChannelMonitor is stale compared to the current ChannelManager! This indicates a potentially-critical violation of the chain::Watch API!");
                                                log_error!(logger, " The ChannelMonitor for channel {} is at update_id {} with update_id through {} in-flight",
                                                        chan.context.channel_id(), monitor.get_latest_update_id(), max_in_flight_update_id);
@@ -10936,7 +10936,7 @@ where
                                                log_error!(logger, " client applications must ensure that ChannelMonitor data is always available and the latest to avoid funds loss!");
                                                log_error!(logger, " Without the latest ChannelMonitor we cannot continue without risking funds.");
                                                log_error!(logger, " Please ensure the chain::Watch API requirements are met and file a bug report at https://github.com/lightningdevkit/rust-lightning");
-                                               return Err(DecodeError::InvalidValue);
+                                               return Err(DecodeError::DangerousValue);
                                        }
                                } else {
                                        // We shouldn't have persisted (or read) any unfunded channel types so none should have been

diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 19c92e5746e51696f658d220cb2ab4fa4964cbfd..f5a36286cfd0d87e577e4c94b29c89b6655eeade 100644 (file)
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -91,6 +91,16 @@ pub enum DecodeError {
        Io(io::ErrorKind),
        /// The message included zlib-compressed values, which we don't support.
        UnsupportedCompression,
+       /// Value is validly encoded but is dangerous to use.
+       ///
+       /// This is used for things like [`ChannelManager`] deserialization where we want to ensure
+       /// that we don't use a [`ChannelManager`] which is in out of sync with the [`ChannelMonitor`].
+       /// This indicates that there is a critical implementation flaw in the storage implementation
+       /// and it's unsafe to continue.
+       ///
+       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
+       /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
+       DangerousValue,
 }
 
 /// An [`init`] message to be sent to or received from a peer.
@@ -1860,6 +1870,7 @@ impl fmt::Display for DecodeError {
                        DecodeError::BadLengthDescriptor => f.write_str("A length descriptor in the packet didn't describe the later data correctly"),
                        DecodeError::Io(ref e) => fmt::Debug::fmt(e, f),
                        DecodeError::UnsupportedCompression => f.write_str("We don't support receiving messages with zlib-compressed fields"),
+                       DecodeError::DangerousValue => f.write_str("Value would be dangerous to continue execution with"),
                }
        }
 }

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 535f9a48667091a86d8c433449bd5e7a439f645d..cfa32a009fe5f9213be2f250b2e033473b16c1b2 100644 (file)
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -1553,6 +1553,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                                                                                                }
                                                                                                (msgs::DecodeError::BadLengthDescriptor, _) => return Err(PeerHandleError { }),
                                                                                                (msgs::DecodeError::Io(_), _) => return Err(PeerHandleError { }),
+                                                                                               (msgs::DecodeError::DangerousValue, _) => return Err(PeerHandleError { }),
                                                                                        }
                                                                                }
                                                                        };

diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs
index 4422af1e8c70c63a81b93ece1aac38e2553d00a0..fa216bc15f4355f2b9b7fcd55cf844061451c1db 100644 (file)
--- a/lightning/src/ln/reload_tests.rs
+++ b/lightning/src/ln/reload_tests.rs
@@ -412,7 +412,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
        }
 
        let mut nodes_0_read = &nodes_0_serialized[..];
-       if let Err(msgs::DecodeError::InvalidValue) =
+       if let Err(msgs::DecodeError::DangerousValue) =
                <(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
                default_config: UserConfig::default(),
                entropy_source: keys_manager,