From 78df66df2f541a9c33e05c9d57596e920f207dd6 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Mon, 13 Apr 2026 21:04:15 +0200 Subject: [PATCH] Validate HTTPS scheme in LSPS5 URL Readable deserialization The `Readable` implementations for `LSPSUrl` and `LSPS5WebhookUrl` were bypassing URL validation, allowing non-HTTPS URLs (e.g., http://, ftp://) to be deserialized from the wire protocol without rejection. Only the serde `Deserialize` and `new()`/`parse()` paths were correctly validating the HTTPS scheme. Route `LSPSUrl::Readable` through `LSPSUrl::parse()` and add a length check to `LSPS5WebhookUrl::Readable` so that wire-deserialized URLs receive the same validation as JSON-deserialized ones. Fixes #4559 Reported-by: Thomas Kilbride of Block Security Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning-liquidity/src/lsps5/msgs.rs | 58 +++++++++++++++++++++- lightning-liquidity/src/lsps5/url_utils.rs | 10 ++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/lightning-liquidity/src/lsps5/msgs.rs b/lightning-liquidity/src/lsps5/msgs.rs index 363a3255f92..6e9c5df1139 100644 --- a/lightning-liquidity/src/lsps5/msgs.rs +++ b/lightning-liquidity/src/lsps5/msgs.rs @@ -457,7 +457,11 @@ impl Writeable for LSPS5WebhookUrl { impl Readable for LSPS5WebhookUrl { fn read(reader: &mut R) -> Result { - Ok(Self(Readable::read(reader)?)) + let url: LSPSUrl = Readable::read(reader)?; + if url.url().len() > MAX_WEBHOOK_URL_LENGTH { + return Err(DecodeError::InvalidValue); + } + Ok(Self(url)) } } @@ -902,6 +906,58 @@ mod tests { } } + #[test] + fn test_lsps_url_readable_rejects_http() { + use lightning::util::ser::Writeable; + + let raw = + lightning_types::string::UntrustedString("http://example.com/webhook".to_string()); + let encoded = raw.encode(); + let result = LSPSUrl::read(&mut lightning::io::Cursor::new(&encoded)); + assert!(result.is_err(), "LSPSUrl::Readable should reject http:// URLs"); + } + + #[test] + fn test_lsps_url_readable_accepts_https() { + use lightning::util::ser::Writeable; + + let https_url = LSPSUrl::parse("https://example.com/webhook".to_string()).unwrap(); + let encoded = https_url.encode(); + let decoded = LSPSUrl::read(&mut lightning::io::Cursor::new(&encoded)).unwrap(); + assert_eq!(decoded.url(), "https://example.com/webhook"); + } + + #[test] + fn test_webhook_url_readable_rejects_http() { + use lightning::util::ser::Writeable; + + let raw = + lightning_types::string::UntrustedString("http://example.com/webhook".to_string()); + let encoded = raw.encode(); + let result = LSPS5WebhookUrl::read(&mut lightning::io::Cursor::new(&encoded)); + assert!(result.is_err(), "Readable should reject http:// webhook URLs"); + } + + #[test] + fn test_webhook_url_readable_rejects_too_long() { + use lightning::util::ser::Writeable; + + let long_url = LSPSUrl::parse(format!("https://example.com/{}", "a".repeat(2000))).unwrap(); + let encoded = long_url.encode(); + let result = LSPS5WebhookUrl::read(&mut lightning::io::Cursor::new(&encoded)); + assert!(result.is_err(), "Readable should reject URLs exceeding MAX_WEBHOOK_URL_LENGTH"); + } + + #[test] + fn test_webhook_url_readable_accepts_valid_https() { + use lightning::util::ser::Writeable; + + let valid_url = LSPS5WebhookUrl::new("https://example.com/webhook".to_string()).unwrap(); + let encoded = valid_url.encode(); + let decoded = LSPS5WebhookUrl::read(&mut lightning::io::Cursor::new(&encoded)).unwrap(); + assert_eq!(decoded.as_str(), "https://example.com/webhook"); + } + #[test] fn test_webhook_notification_parameter_binding() { let notification = WebhookNotification::expiry_soon(144); diff --git a/lightning-liquidity/src/lsps5/url_utils.rs b/lightning-liquidity/src/lsps5/url_utils.rs index 2d49c10ff08..2a660b4495f 100644 --- a/lightning-liquidity/src/lsps5/url_utils.rs +++ b/lightning-liquidity/src/lsps5/url_utils.rs @@ -69,9 +69,12 @@ impl LSPSUrl { Ok(LSPSUrl(UntrustedString(url_str))) } - /// Returns URL length. + /// Returns URL length in bytes. + /// + /// Since [`LSPSUrl::parse`] only accepts ASCII characters, this is equivalent + /// to the character count. pub fn url_length(&self) -> usize { - self.0 .0.chars().count() + self.0 .0.len() } /// Returns the full URL string. @@ -99,6 +102,7 @@ impl Writeable for LSPSUrl { impl Readable for LSPSUrl { fn read(reader: &mut R) -> Result { - Ok(Self(Readable::read(reader)?)) + let s: UntrustedString = Readable::read(reader)?; + Self::parse(s.0).map_err(|_| DecodeError::InvalidValue) } }