Cut 0.2.3/dns-resolver 0.3.1/invoice 0.34.1/types 0.3.2

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).

CHANGELOG.md

@@ -71,7 +71,19 @@
 
 ## Security
 0.2.3 fixes several underestimates of the anchor reserves required to ensure we
-can reliably close channels and a sanitization issue.
+can reliably close channels, several denial-of-service vulnerabilities and a
+sanitization issue.
+ * `Bolt11Invoice::recover_payee_pub_key` no longer panics if called on an
+ invoice which set an explicit public key, rather than relying on public key
+ recovery. Note that this method is called from
+ `PaymentParameters::from_bolt11_invoice` (#4717).
+ * Maliciously-crafted unpayable invoices which have overflowing feerates will
+ no longer cause an `unwrap` failure panic (#4716).
+ * `possiblyrandom` did not properly generate random data except when it was
+ explicitly configured to. By default this means LDK is vulnerable to various
+ HashDoS attacks (#4719).
+ * `OMNameResolver` will no longer panic when looking up payment instructions
+ which include unicode characters at the start of a TXT record (#4718).
  * When using the `anchor_channel_reserves` module to calculate reserves
  required to pay for fees when closing anchor channels, zero-fee-commitment
  channels were not considered. This could allow a counterparty to open many
@@ -80,6 +92,13 @@ can reliably close channels and a sanitization issue.
  the wallet by ignoring the `TxIn` cost to spend them (#4670).
  * `PrintableString` did not properly sanitize unicode format characters,
  allowing an attacker to corrupt the rendering of logs or UI (#4593, #4605).
+ * RGS data is now limited in how large of a graph it is able to cause a client
+ to store in memory. Note that RGS data is still considered a DoS vector in
+ general and you should only use semi-trusted RGS data (#4713).
+ * Counterparty-provided strings in failure messages are no longer logged in
+ full, reducing the ability of such a counterparty to spam our logs (#4714).
+ * Reading a corrupted `ChannelManager` or `ProbabilisticScorer` can no longer
+ cause us to allocate large amounts of memory (#4712).
 
 Thanks to Project Loupe for reporting most of the issues fixed in this release.
 

lightning-background-processor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-custom-message/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-dns-resolver/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-dns-resolver"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning-invoice/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.34.0"
+version = "0.34.1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"

lightning-invoice/src/lib.rs

@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
  self.signed_invoice.features()
 	}
 
-	/// Recover the payee's public key (only to be used if none was included in the invoice)
+	/// Get the invoice's payee public key.
+	///
+	/// This uses the explicitly included payee public key, if present, otherwise it recovers the
+	/// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
 	pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
 	}
 
-	/// Recover the payee's public key if one was included in the invoice, otherwise return the
-	/// recovered public key from the signature
+	/// Get the invoice's payee public key, preferring an explicitly included payee public key and
+	/// falling back to recovering the key from the signature.
 	pub fn get_payee_pub_key(&self) -> PublicKey {
  match self.payee_pub_key() {
  Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
  }
 	}
 
@@ -2044,6 +2049,44 @@ mod test {
  assert!(new_signed.check_signature());
 	}
 
+	#[test]
+	fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::*;
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+ let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
+
+ let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(sha256::Hash::from_slice(&[0; 32][..]).unwrap())
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let signed_raw = invoice.into_signed_raw();
+ let (raw_invoice, hash, signature) = signed_raw.into_parts();
+ let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
+ let bad_rid = RecoveryId::from_i32(2).unwrap();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
+
+ assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
+ assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
+	}
+
 	#[test]
 	fn test_check_feature_bits() {
  use crate::TaggedField::*;

lightning-liquidity/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-liquidity"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["John Cantrell <johncantrell97@gmail.com>", "Elias Rohrer <dev@tnull.de>"]
 homepage = "https://lightningdevkit.org/"
 license = "MIT OR Apache-2.0"

lightning-persister/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.2.0"
+version = "0.2.3"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"

lightning-rapid-gossip-sync/src/lib.rs

@@ -153,6 +153,10 @@ where
 	/// Sync gossip data from a file.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `network_graph`: The network graph to apply the updates to
 	///
 	/// `sync_path`: Path to the file where the gossip update data is located
@@ -172,6 +176,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	#[cfg(feature = "std")]
 	pub fn update_network_graph(&self, update_data: &[u8]) -> Result<u32, GraphSyncError> {
@@ -182,6 +190,10 @@ where
 	/// Update network graph from binary data.
 	/// Returns the last sync timestamp to be used the next time rapid sync data is queried.
 	///
+	/// You should consider the gossip data source as semi-trusted. It is generally the case that it
+	/// can DoS the client either by omitting data which leads to pathfinding failure or by bloating
+	/// the graph such that it leads to eventual OOM on the client.
+	///
 	/// `update_data`: `&[u8]` binary stream that comprises the update data
 	/// `current_time_unix`: `Option<u64>` optional current timestamp to verify data age
 	pub fn update_network_graph_no_std(

lightning-rapid-gossip-sync/src/processing.rs

@@ -9,7 +9,9 @@ use lightning::ln::msgs::{
 	DecodeError, ErrorAction, LightningError, SocketAddress, UnsignedChannelUpdate,
 	UnsignedNodeAnnouncement,
 };
-use lightning::routing::gossip::{NetworkGraph, NodeAlias, NodeId};
+use lightning::routing::gossip::{
+	NetworkGraph, NodeAlias, NodeId, CHAN_COUNT_ESTIMATE, NODE_COUNT_ESTIMATE,
+};
 use lightning::util::logger::Logger;
 use lightning::util::ser::{BigSize, FixedLengthReader, Readable};
 use lightning::{log_debug, log_given_level, log_gossip, log_trace, log_warn};
@@ -115,17 +117,27 @@ where
  }
  };
 
+ const MAX_NODE_COUNT: u32 = (NODE_COUNT_ESTIMATE as u32) * 10;
+ const MAX_CHANNEL_COUNT: u64 = (CHAN_COUNT_ESTIMATE as u64) * 10;
+
  let node_id_count: u32 = Readable::read(read_cursor)?;
+ if node_id_count > MAX_NODE_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of nodes to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  let mut node_ids: Vec<NodeId> = Vec::with_capacity(core::cmp::min(
  node_id_count,
  MAX_INITIAL_NODE_ID_VECTOR_CAPACITY,
  ) as usize);
-
  let network_graph = &self.network_graph;
  let mut node_modifications: Vec<UnsignedNodeAnnouncement> = Vec::new();
 
+ let read_only_network_graph = network_graph.read_only();
+
  if parse_node_details {
- let read_only_network_graph = network_graph.read_only();
  for _ in 0..node_id_count {
  let mut pubkey_bytes = [0u8; 33];
  read_cursor.read_exact(&mut pubkey_bytes)?;
@@ -237,9 +249,12 @@ where
  }
  }
 
+ let original_graph_channel_count = read_only_network_graph.channels().len() as u32;
+ core::mem::drop(read_only_network_graph);
+
  let mut previous_scid: u64 = 0;
  let announcement_count: u32 = Readable::read(read_cursor)?;
- for _ in 0..announcement_count {
+ for i in 0..announcement_count {
  let features = Readable::read(read_cursor)?;
 
  // handle SCID
@@ -284,6 +299,10 @@ where
  }
  }
 
+ if (original_graph_channel_count as u64) + (i as u64) > MAX_CHANNEL_COUNT {
+ continue;
+ }
+
  let announcement_result = network_graph.add_channel_from_partial_announcement(
  short_channel_id,
  funding_sats,
@@ -329,6 +348,13 @@ where
  previous_scid = 0;
 
  let update_count: u32 = Readable::read(read_cursor)?;
+ if update_count as u64 > MAX_CHANNEL_COUNT {
+ return Err(LightningError {
+ err: "RGS data contained nonsense number of channels to update".to_owned(),
+ action: ErrorAction::IgnoreError,
+ }
+ .into());
+ }
  log_debug!(self.logger, "Processing RGS update from {} with {} nodes, {} channel announcements and {} channel updates.",
  latest_seen_timestamp, node_id_count, announcement_count, update_count);
  if update_count == 0 {

lightning-types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning-types"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.2.2"
+version = "0.2.3"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"

lightning/src/ln/channel.rs

@@ -1959,9 +1959,12 @@ where
 
  let tx_abort = should_ack.then(|| {
  let logger = WithChannelContext::from(logger, &self.context(), None);
- let reason =
- types::string::UntrustedString(String::from_utf8_lossy(&msg.data).to_string());
- log_info!(logger, "Counterparty failed interactive transaction negotiation: {reason}");
+ let reason = String::from_utf8_lossy(&msg.data);
+ log_info!(
+ logger,
+ "Counterparty failed interactive transaction negotiation: {}",
+ log_msg!(reason)
+ );
  msgs::TxAbort {
  channel_id: msg.channel_id,
  data: "Acknowledged tx_abort".to_string().into_bytes(),

lightning/src/ln/invoice_utils.rs

@@ -1288,7 +1288,7 @@ mod test {
  assert!(!invoice.features().unwrap().supports_basic_mpp());
 
  let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  invoice.min_final_cltv_expiry_delta() as u32,
  )
  .with_bolt11_features(invoice.features().unwrap().clone())
@@ -1350,7 +1350,7 @@ mod test {
  payment_secret,
  payment_amt,
  payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
  );
  do_claim_payment_along_route(ClaimAlongRouteArgs::new(
  &nodes[0],

lightning/src/ln/peer_handler.rs

@@ -45,7 +45,6 @@ use crate::onion_message::packet::OnionMessageContents;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::sign::{NodeSigner, Recipient};
 use crate::types::features::{InitFeatures, NodeFeatures};
-use crate::types::string::PrintableString;
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::ser::{VecWriter, Writeable, Writer};
@@ -2415,7 +2414,7 @@ where
  logger,
  "Got Err message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  self.message_handler.chan_handler.handle_error(their_node_id, &msg);
  if msg.channel_id.is_zero() {
@@ -2427,7 +2426,7 @@ where
  logger,
  "Got warning message from {}: {}",
  their_node_id,
-PrintableString(&msg.data)
+log_msg!(msg.data)
  );
  },
 
@@ -3213,7 +3212,7 @@ where
  msgs::ErrorAction::DisconnectPeer { msg } => {
  if let Some(msg) = msg.as_ref() {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  } else {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {}",
  node_id);
@@ -3228,7 +3227,7 @@ where
  },
  msgs::ErrorAction::DisconnectPeerWithWarning { msg } => {
  log_trace!(logger, "Handling DisconnectPeer HandleError event in peer_handler for node {} with message {}",
- node_id, msg.data);
+ node_id, log_msg!(msg.data));
  // We do not have the peers write lock, so we just store that we're
  // about to disconnect the peer and do it after we finish
  // processing most messages.
@@ -3254,7 +3253,7 @@ where
  msgs::ErrorAction::SendErrorMessage { ref msg } => {
  log_trace!(logger, "Handling SendErrorMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,
@@ -3266,7 +3265,7 @@ where
  } => {
  log_given_level!(logger, *log_level, "Handling SendWarningMessage HandleError event in peer_handler for node {} with message {}",
  node_id,
- msg.data);
+log_msg!(msg.data));
  self.enqueue_message(
  &mut *get_peer_for_forwarding!(&node_id)?,
  msg,

lightning/src/onion_message/dns_resolution.rs

@@ -520,7 +520,8 @@ impl OMNameResolver {
  .filter_map(|data| String::from_utf8(data).ok())
  .filter(|data_string| data_string.len() > URI_PREFIX.len())
  .filter(|data_string| {
- data_string[..URI_PREFIX.len()].eq_ignore_ascii_case(URI_PREFIX)
+ let pfx = &data_string.as_bytes()[..URI_PREFIX.len()];
+ pfx.eq_ignore_ascii_case(URI_PREFIX.as_bytes())
  });
  // Check that there is exactly one TXT record that begins with
  // bitcoin: as required by BIP 353 (and is valid UTF-8).