Skip to content

Add DecodeError::DangerousValue for decoding invalid channel managers #2974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fuzz/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}}
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10918,7 +10918,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);
Expand All @@ -10927,7 +10927,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
Expand Down
11 changes: 11 additions & 0 deletions lightning/src/ln/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -1796,6 +1806,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"),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/peer_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,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 { }),
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/reload_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading