From 0de876db9ec7b9abf3eefa61f4951ff9330c592a Mon Sep 17 00:00:00 2001 From: t-bast Date: Mon, 20 Jul 2026 14:44:27 +0200 Subject: [PATCH] Ban unicode control characters from utf8 fields We ban `NULL` and other control characters in utf8 fields: there's no valid use-case for them. We also make sure that feature bits MUST be minimally-encoded everywhere instead of being lenient for no good reason. See https://github.com/lightning/bolts/pull/1341 --- .../main/scala/fr/acinq/eclair/Features.scala | 3 + .../scala/fr/acinq/eclair/NodeParams.scala | 1 + .../fr/acinq/eclair/io/PeerConnection.scala | 4 ++ .../acinq/eclair/payment/Bolt11Invoice.scala | 2 + .../acinq/eclair/payment/Bolt12Invoice.scala | 2 + .../remote/EclairInternalsSerializer.scala | 2 + .../scala/fr/acinq/eclair/router/Router.scala | 2 + .../fr/acinq/eclair/router/Validation.scala | 13 ++++ .../eclair/wire/protocol/CommonCodecs.scala | 12 ++++ .../eclair/wire/protocol/OfferTypes.scala | 5 ++ .../scala/fr/acinq/eclair/FeaturesSpec.scala | 14 +++++ .../scala/fr/acinq/eclair/StartupSpec.scala | 18 ++++++ .../eclair/payment/Bolt11InvoiceSpec.scala | 1 - .../EclairInternalsSerializerSpec.scala | 2 + .../fr/acinq/eclair/router/RouterSpec.scala | 48 +++++++++++++++ .../wire/protocol/CommonCodecsSpec.scala | 26 ++++++++ .../protocol/LightningMessageCodecsSpec.scala | 61 +++++++++++++++++++ 17 files changed, 215 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala index de93ef08a8..d2e7a851b1 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Features.scala @@ -135,6 +135,9 @@ case class Features[T <: Feature](activated: Map[T, FeatureSupport], encoded_opt def isEmpty: Boolean = activated.isEmpty && encoded_opt.forall(_.isEmpty) + /** The `features` field of lightning messages must be minimally-encoded: it must not have a leading zero byte. */ + def isMinimallyEncoded: Boolean = encoded_opt.forall(e => e.bin.isEmpty || e.bin(0) != 0) + def hasFeature(feature: T, support: Option[FeatureSupport] = None): Boolean = support match { case Some(s) => activated.get(feature).contains(s) || encoded_opt.exists(_.hasFeature(feature, support)) case None => activated.contains(feature) || encoded_opt.exists(_.hasFeature(feature)) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala index c1a695a80f..635105b03d 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala @@ -380,6 +380,7 @@ object NodeParams extends Logging { val nodeAlias = config.getString("node-alias") require(nodeAlias.getBytes("UTF-8").length <= 32, "invalid alias, too long (max allowed 32 bytes)") + require(CommonCodecs.isValidUtf8(nodeAlias), "invalid alias, control characters are not allowed") def validateFeatures(features: Features[Feature]): Unit = { val featuresErr = Features.validateFeatureGraph(features) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala index 4a81d38cc4..496741f094 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala @@ -492,6 +492,10 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A // other rejections are not considered punishable offenses // we are not using a catch-all on purpose, to make compiler warn us when a new error is added case _: GossipDecision.Duplicate => d.behavior + // The announcement may have been relayed by this peer on behalf of another node: we don't punish them for + // invalid content that was created by a third party. + case _: GossipDecision.InvalidFeatures => d.behavior + case _: GossipDecision.InvalidAlias => d.behavior case _: GossipDecision.NoKnownChannel => d.behavior case _: GossipDecision.ValidationFailure => d.behavior case _: GossipDecision.ChannelPruned => d.behavior diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala index 36bd6fb9f9..47d6f4e9ec 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala @@ -19,6 +19,7 @@ package fr.acinq.eclair.payment import fr.acinq.bitcoin.scalacompat.Crypto.{PrivateKey, PublicKey} import fr.acinq.bitcoin.scalacompat.{Block, BlockHash, ByteVector32, ByteVector64, Crypto} import fr.acinq.bitcoin.{Base58, Base58Check, Bech32} +import fr.acinq.eclair.wire.protocol.CommonCodecs import fr.acinq.eclair.{Bolt11Feature, CltvExpiryDelta, Feature, FeatureSupport, Features, InvoiceFeature, MilliSatoshi, MilliSatoshiLong, ShortChannelId, TimestampSecond, randomBytes32} import scodec.bits.{BitVector, ByteOrdering, ByteVector} import scodec.codecs.{list, ubyte} @@ -46,6 +47,7 @@ case class Bolt11Invoice(prefix: String, amount_opt: Option[MilliSatoshi], creat amount_opt.foreach(a => require(a > 0.msat, s"amount is not valid")) require(tags.collect { case _: Bolt11Invoice.PaymentHash => }.size == 1, "there must be exactly one payment hash tag") require(tags.collect { case Bolt11Invoice.Description(_) | Bolt11Invoice.DescriptionHash(_) => }.size == 1, "there must be exactly one description tag or one description hash tag") + require(tags.collect { case Bolt11Invoice.Description(d) => d }.forall(d => CommonCodecs.isValidUtf8(d)), "invoice description must be a valid UTF-8 string") require(tags.collect { case _: Bolt11Invoice.PaymentSecret => }.size == 1, "there must be exactly one payment secret tag") require(tags.collect { case f: Bolt11Invoice.FallbackAddress => Try(FallbackAddress(Bolt11Invoice.FallbackAddress.toAddress(f, prefix))) }.forall(_.isSuccess), "invalid fallback address") require(Features.validateFeatureGraph(features).isEmpty, Features.validateFeatureGraph(features).map(_.message)) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt12Invoice.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt12Invoice.scala index 48e0ea66b3..956e6f9269 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt12Invoice.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt12Invoice.scala @@ -63,6 +63,8 @@ case class Bolt12Invoice(records: TlvStream[InvoiceTlv]) extends Invoice { Left("Invoice expired") } else if (request.amount != amount) { Left("Incompatible amount") + } else if (!features.isMinimallyEncoded) { + Left("Non minimally-encoded features") } else if (!Features.areCompatible(request.features, features.bolt12Features())) { Left("Incompatible features") } else if (!checkSignature()) { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/remote/EclairInternalsSerializer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/remote/EclairInternalsSerializer.scala index 22ca8235d3..2d90b66c70 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/remote/EclairInternalsSerializer.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/remote/EclairInternalsSerializer.scala @@ -227,5 +227,7 @@ object EclairInternalsSerializer { .typecase(51, lengthPrefixedChannelAnnouncementCodec.as[GossipDecision.ChannelClosed]) .typecase(52, peerConnectionKillCodec) .typecase(53, peerConnectionDoSyncCodec) + .typecase(54, lengthPrefixedAnnouncementCodec.as[GossipDecision.InvalidFeatures]) + .typecase(55, lengthPrefixedNodeAnnouncementCodec.as[GossipDecision.InvalidAlias]) } \ No newline at end of file diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala index ad1a48d359..73a6e4d927 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala @@ -788,6 +788,8 @@ object Router { sealed trait Rejected extends GossipDecision case class Duplicate(ann: AnnouncementMessage) extends Rejected case class InvalidSignature(ann: AnnouncementMessage) extends Rejected + case class InvalidFeatures(ann: AnnouncementMessage) extends Rejected + case class InvalidAlias(ann: NodeAnnouncement) extends Rejected case class NoKnownChannel(ann: NodeAnnouncement) extends Rejected case class ValidationFailure(ann: ChannelAnnouncement) extends Rejected case class InvalidAnnouncement(ann: ChannelAnnouncement) extends Rejected diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala index fe429a8ee9..cf6d2d0079 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala @@ -66,6 +66,11 @@ object Validation { log.debug("ignoring {} (was pruned)", c) sendDecision(origin.peerConnection, GossipDecision.ChannelPruned(c)) d + } else if (!c.features.isMinimallyEncoded) { + origin.peerConnection ! TransportHandler.ReadAck(c) + log.debug("ignoring {} (features are not minimally-encoded)", c) + sendDecision(origin.peerConnection, GossipDecision.InvalidFeatures(c)) + d } else if (!Announcements.checkSigs(c)) { origin.peerConnection ! TransportHandler.ReadAck(c) log.warning("bad signature for announcement {}", c) @@ -340,6 +345,14 @@ object Validation { log.debug("ignoring {} (duplicate)", n) remoteOrigins.foreach(sendDecision(_, GossipDecision.Duplicate(n))) d + } else if (!n.features.isMinimallyEncoded) { + log.debug("ignoring {} (features are not minimally-encoded)", n) + remoteOrigins.foreach(sendDecision(_, GossipDecision.InvalidFeatures(n))) + d + } else if (!CommonCodecs.isValidUtf8(n.alias)) { + log.debug("ignoring {} (invalid alias)", n) + remoteOrigins.foreach(sendDecision(_, GossipDecision.InvalidAlias(n))) + d } else if (!Announcements.checkSig(n)) { log.warning("bad signature for {}", n) remoteOrigins.foreach(sendDecision(_, GossipDecision.InvalidSignature(n))) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala index 9feb0093ed..b0ea959724 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala @@ -170,6 +170,18 @@ object CommonCodecs { val txCodec: Codec[Transaction] = bytes.xmap(d => Transaction.read(d.toArray), d => Transaction.write(d)) + /** + * BOLT 1: a `utf8` field must not contain characters in the unicode "Other" (C*) General Category. This bans control + * characters (Cc), format characters (Cf), surrogates (Cs), private-use characters (Co) and unassigned code points + * (Cn), which have no valid use-case in lightning messages and are a source of interoperability issues. + * Note that the spec only mandates rejecting Cc/Cf/Cs/Co when reading, but we also reject Cn (unassigned) both when + * reading and writing. + */ + def isValidUtf8(s: String): Boolean = { + val prohibited = Set(Character.CONTROL, Character.FORMAT, Character.SURROGATE, Character.PRIVATE_USE, Character.UNASSIGNED).map(_.toInt) + !s.codePoints().anyMatch(codePoint => prohibited.contains(Character.getType(codePoint))) + } + def zeropaddedstring(size: Int): Codec[String] = fixedSizeBytes(size, utf8).xmap(s => s.takeWhile(_ != '\u0000'), s => s) /** diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/OfferTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/OfferTypes.scala index d1717a1985..8335dc331b 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/OfferTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/OfferTypes.scala @@ -314,6 +314,9 @@ object OfferTypes { def validate(records: TlvStream[OfferTlv]): Either[InvalidTlvPayload, Offer] = { if (records.get[OfferDescription].isEmpty && records.get[OfferAmount].nonEmpty) return Left(MissingRequiredTlv(UInt64(10))) + if (records.get[OfferDescription].exists(d => !CommonCodecs.isValidUtf8(d.description))) return Left(ForbiddenTlv(UInt64(10))) + if (records.get[OfferIssuer].exists(i => !CommonCodecs.isValidUtf8(i.issuer))) return Left(ForbiddenTlv(UInt64(18))) + if (records.get[OfferFeatures].exists(_.features.headOption.contains(0))) return Left(ForbiddenTlv(UInt64(12))) if (records.get[OfferAmount].exists(_.amount == 0)) return Left(ForbiddenTlv(UInt64(10))) if (records.get[OfferNodeId].isEmpty && records.get[OfferPaths].isEmpty) return Left(MissingRequiredTlv(UInt64(22))) if (records.get[OfferCurrency].nonEmpty && records.get[OfferAmount].isEmpty) return Left(MissingRequiredTlv(UInt64(8))) @@ -432,6 +435,8 @@ object OfferTypes { if (records.get[InvoiceRequestMetadata].isEmpty) return Left(MissingRequiredTlv(UInt64(0))) if (records.get[InvoiceRequestAmount].isEmpty && records.get[OfferAmount].isEmpty) return Left(MissingRequiredTlv(UInt64(82))) if (records.get[InvoiceRequestPayerId].isEmpty) return Left(MissingRequiredTlv(UInt64(88))) + if (records.get[InvoiceRequestFeatures].exists(_.features.headOption.contains(0))) return Left(ForbiddenTlv(UInt64(84))) + if (records.get[InvoiceRequestPayerNote].exists(n => !CommonCodecs.isValidUtf8(n.note))) return Left(ForbiddenTlv(UInt64(89))) if (records.get[Signature].isEmpty) return Left(MissingRequiredTlv(UInt64(240))) if (records.unknown.exists(!isInvoiceRequestTlv(_))) return Left(ForbiddenTlv(records.unknown.find(!isInvoiceRequestTlv(_)).get.tag)) Right(InvoiceRequest(records)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala index 357b84c705..c94a8f65c5 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/FeaturesSpec.scala @@ -303,6 +303,20 @@ class FeaturesSpec extends AnyFunSuite { } } + test("features minimal encoding") { + // Features that we build ourselves are always minimally-encoded. + assert(Features.empty.isMinimallyEncoded) + assert(Features(VariableLengthOnion -> Mandatory).isMinimallyEncoded) + assert(Features(Map.empty[Feature, FeatureSupport], Some(EncodedFeatures.fromFeatureBits(Set(151, 160, 175)))).isMinimallyEncoded) + // Features decoded from the wire are minimally-encoded when they don't have a leading zero byte. + assert(Features(hex"").isMinimallyEncoded) + assert(Features(hex"80").isMinimallyEncoded) + assert(Features(hex"0100").isMinimallyEncoded) + assert(!Features(hex"00").isMinimallyEncoded) + assert(!Features(hex"0080").isMinimallyEncoded) + assert(!Features(hex"000100").isMinimallyEncoded) + } + test("parse features from configuration") { { val conf = ConfigFactory.parseString( diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala index 805f9f22e0..ea5c8dadb6 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala @@ -71,6 +71,24 @@ class StartupSpec extends AnyFunSuite { assert(nodeParamsAttempt.isFailure && nodeParamsAttempt.failed.get.getMessage.contains("alias, too long")) } + test("NodeParams should fail if the alias contains control characters") { + val illegalAliases = Seq( + "alice" + 0x0000.toChar, // NULL (Cc) + "alice" + 0x0007.toChar, // BEL (Cc) + "alice" + 0x200b.toChar, // zero-width space (Cf) + s"${0xfeff.toChar}alice", // zero-width no-break space / BOM (Cf) + ) + illegalAliases.foreach { alias => + val conf = ConfigFactory.parseMap(Map("node-alias" -> alias).asJava).withFallback(defaultConf) + val nodeParamsAttempt = Try(makeNodeParamsWithDefaults(conf)) + assert(nodeParamsAttempt.isFailure && nodeParamsAttempt.failed.get.getMessage.contains("control characters")) + } + // A valid alias with multi-byte UTF-8 characters (including a supplementary code point) is accepted. + val validAlias = "alice " + new String(Character.toChars(0x1f004)) // mahjong tile red dragon (So) + val validConf = ConfigFactory.parseMap(Map("node-alias" -> validAlias).asJava).withFallback(defaultConf) + assert(Try(makeNodeParamsWithDefaults(validConf)).isSuccess) + } + test("NodeParams should fail with deprecated global-features, local-features or hex features") { for (deprecated <- Seq("global-features", "local-features")) { val illegalGlobalFeaturesConf = ConfigFactory.parseString(deprecated + " = \"0200\"") diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/Bolt11InvoiceSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/Bolt11InvoiceSpec.scala index c307206af4..836917029e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/Bolt11InvoiceSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/Bolt11InvoiceSpec.scala @@ -638,7 +638,6 @@ class Bolt11InvoiceSpec extends AnyFunSuite { "lnbc10n1pwykdacsp5glv27qqqx54sn7jn2f3szsc64celf7dpf5cvyh7vf2e242ceh4vqpp5kegv2kdkxmetm2tpnzfgt4640n7mgxl95jnpc6fkz6uyjdwahw8sdpstfshq5n9v9jzucm0d5s8vmm5v5s8qmmnwssyj3p6yqenwdp5cqzysxqrrssjlny2skwtnnese9nmw99xlh7jwgtdxurhce2zcwsamevmj37kd5yzxzu55mt567seewmajra2hwyry5cv9kfzf02paerhs7tf9acdcgqva6k7j" -> PublicKey(hex"020e3d0c063f7442b16a1f982dcc343dbe2a81e85b664f9dc2645965c715bfbfe3"), "lnbc3100n1pwp370ssp5tcrvxtavpnk80w3z3pq53zlwqhdu2nc8yjk2k3qsn223sa4t5pdqpp5ku7y6tfz5up840v00vgc2vmmqtpsu5ly98h09vxv9d7k9xtq8mrsdpjd35kw6r5de5kueevypkxjemgw3hxjmn89ssxc6t8dp6xu6twvucqp2sunrt8slx2wmvjzdv3vvlls9gez7g2gd37g2pwa4pnlswuxzy0w3hd5kkqdrpl4ylcdhvkvuamwjsfh79nkn52dq0qpzj8c4rf57jmgqe864ex" -> PublicKey(hex"03460feb831a3b31f2004b3c8484012e4a10f35b4f6a8f97f9711706c37c91bcaf"), "lnbc1500n1pwr7z8rsp500swxekcn6psptyjztm22w06atutqx759xmypmqjcsg8p25n25pqpp5hyfkmnwwx7x902ys52du8pph6hdkarnqvj6fwhh9swfsg5lp94vsdpa2fjkzep6ypph2um5dajxjctvypmkzmrvv468xgrpwfjjqetkd9kzqctwvss8ycqzysxqr23s64a2h7gn25pchh8r6jpe236h925fylw2jcm4pd92w8hkmpflreph8r6s8jnnml0zu47qv6t2sj6frnle2cpanf6e027vsddgkl8hk7gpw8209n" -> PublicKey(hex"03044dd84ef699fc273e02a4a459ba2acfebfcc05f09bdd54d24e9664603bffc28"), - "lnbc1500n1pdl05v0sp5c44hx2kt6hsslqn4yv2wc67vgz9upwvmhmxw5d4vqhv9edrnm50qpp5c4t5p3renelctlh0z4jpznyxna7lw9zhws868wktp8vtn8t5a8uqdpa2fjkzep6ypxxjemgw35kueeqfejhgam0wf4jqnrfw96kjerfw3ujq5r0dakq6cqzysxqr23s7k3ktaae69gpl2tfleyy2rsm0m6cy5yvf8uq7g4dmpyrwvfxzslnvryx5me4xh0fsp9jfjsqkuwpzx9ydwe6ndrm0eznarhdrfwn5gsph3ulpz" -> PublicKey(hex"0386b9ebc56df241424a052ec92dd6eff5b833275bb84376ffa6780c269fe322f8"), "lnbc1500n1pwyvxp3sp55tutdhtnzx2kv66v0p0knc7wfup8jkrxqvujjc3fzlsh80slr2lqpp5ch8jx4g0ft0f6tzg008vr82wv92sredy07v46h7q3h3athx2nm2sdpa2fjkzep6ypyx7aeqfys8w6tndqsx67fqw35x2gzvv4jxwetjypvzqam0w4kxgcqzysxqr23s3hdgx90a6jcqgl84z36dv6kn6eg4klsaje2kdm84662rq7lzzzlycvne4l8d0steq5pctdp4ffeyhylgrt7ln92l8dyvrnsn9qg5qkgqvffgf6" -> PublicKey(hex"02dc6c9deef8f6033e24d6a9653902b68863086d8aedf395f0fc90c42d147538e5"), "lnbc1500n1pwr7z2psp55gly6gsxyqz4mfhk08el8s0amucxzgnd7kfec85gk7vrx0fjmpdqpp5cuzt0txjkkmpz6sgefdjjmdrsj9gl8fqyeu6hx7lj050f68yuceqdqvg9jxgg8zn2sscqzysxqr23s7442lgk6cj95qygw2hly9qw9zchhag5p5m3gyzrmws8namcsqh5nz2nm6a5sc2ln6jx59sln9a7t8vxtezels2exurr0gchz9gk0ufgp8rkm46" -> PublicKey(hex"02a41788407d9a013dd43c3324be954e962abb585906b39cfa7e27c85d7e1605f2"), "lnbc1500n1pd7u7g4sp57svjn6jgv6jtxz9ezh96uwr8dykqyz55nlldp4rn7g7ms07ust8spp5eam7uhxc0w4epnuflgkl62m64qu378nnhkg3vahkm7dhdcqnzl4sdqvg9jxgg8zn2sscqzysxqr23s870l2549nhsr2dfv9ehkl5z95p5rxpks5j2etr35e02z9r6haalrfjs7sz5y7wzenywp8t52w89c9u8taf9m76t2p0w0vxw243y7l4sp5y7gz6" -> PublicKey(hex"026792244df8b138c2000c92ba99dbbf06cea08d06764a1919726b0df3a999e629"), diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/remote/EclairInternalsSerializerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/remote/EclairInternalsSerializerSpec.scala index 842e84e640..ddcc174ad3 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/remote/EclairInternalsSerializerSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/remote/EclairInternalsSerializerSpec.scala @@ -27,6 +27,8 @@ class EclairInternalsSerializerSpec extends AnyFunSuite { case _: GossipDecision.Accepted => () case _: GossipDecision.Duplicate => () case _: GossipDecision.InvalidSignature => () + case _: GossipDecision.InvalidFeatures => () + case _: GossipDecision.InvalidAlias => () case _: GossipDecision.NoKnownChannel => () case _: GossipDecision.ValidationFailure => () case _: GossipDecision.InvalidAnnouncement => () diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala index e0540effaa..1905b8ea5f 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouterSpec.scala @@ -93,6 +93,54 @@ class RouterSpec extends BaseRouterSpec { } } + test("reject announcements with an invalid alias or non-minimally-encoded features") { fixture => + import fixture._ + val eventListener = TestProbe() + system.eventStream.subscribe(eventListener.ref, classOf[NetworkEvent]) + system.eventStream.subscribe(eventListener.ref, classOf[Rebroadcast]) + val peerConnection = TestProbe() + + // Flush the announcements that are pending rebroadcast from the fixture setup. + router ! Router.TickBroadcast + eventListener.expectMsgType[Rebroadcast] + + { + // node_announcement with a control character in its alias: we ignore it but keep the connection open. + val node = makeNodeAnnouncement(randomKey(), "invalid\u0007alias", Color(1, 2, 3), Nil, Features.empty) + assert(!CommonCodecs.isValidUtf8(node.alias)) + assert(Announcements.checkSig(node)) // the signature is valid: only the alias content is invalid + peerConnection.send(router, PeerRoutingMessage(peerConnection.ref, remoteNodeId, node)) + peerConnection.expectMsg(TransportHandler.ReadAck(node)) + peerConnection.expectMsg(GossipDecision.InvalidAlias(node)) + } + { + // node_announcement with non-minimally-encoded features: the signature is still valid because we strip the + // leading zero byte before verifying it, so we must explicitly reject the message. + val node = makeNodeAnnouncement(randomKey(), "node", Color(1, 2, 3), Nil, Features.empty) + val invalidNode = node.copy(features = Features(hex"00" ++ node.features.toByteVector)) + assert(!invalidNode.features.isMinimallyEncoded) + assert(Announcements.checkSig(invalidNode)) + peerConnection.send(router, PeerRoutingMessage(peerConnection.ref, remoteNodeId, invalidNode)) + peerConnection.expectMsg(TransportHandler.ReadAck(invalidNode)) + peerConnection.expectMsg(GossipDecision.InvalidFeatures(invalidNode)) + } + { + // channel_announcement with non-minimally-encoded features: we reject it without even validating the funding tx. + val chan = channelAnnouncement(RealShortChannelId(BlockHeight(420000), 105, 0), priv_a, priv_c, priv_funding_a, priv_funding_c) + val invalidChan = chan.copy(features = Features(hex"00" ++ chan.features.toByteVector)) + assert(!invalidChan.features.isMinimallyEncoded) + assert(Announcements.checkSigs(invalidChan)) + peerConnection.send(router, PeerRoutingMessage(peerConnection.ref, remoteNodeId, invalidChan)) + peerConnection.expectMsg(TransportHandler.ReadAck(invalidChan)) + peerConnection.expectMsg(GossipDecision.InvalidFeatures(invalidChan)) + watcher.expectNoMessage(100 millis) + } + + peerConnection.expectNoMessage(100 millis) + router ! Router.TickBroadcast + eventListener.expectNoMessage(100 millis) + } + test("properly announce valid new channels and ignore invalid ones") { fixture => import fixture._ val eventListener = TestProbe() diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/CommonCodecsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/CommonCodecsSpec.scala index ec7cae41b5..659cb74550 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/CommonCodecsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/CommonCodecsSpec.scala @@ -173,6 +173,32 @@ class CommonCodecsSpec extends AnyFunSuite { assert(channelflags.decode(bin"11111100").require == DecodeResult(ChannelFlags(nonInitiatorPaysCommitFees = false, announceChannel = false), BitVector.empty)) } + test("validate utf8 strings (control characters are not allowed)") { + val valid = Seq( + "", + "alice", + "ACINQ", + "\u65e5\u672c\u8a9e", // japanese + "utf8 with an emoji \ud83d\ude80", // rocket (So) + "unicode symbols \u20bf \u2713 \u2605", // bitcoin sign (Sc), check mark and star (So) + ) + valid.foreach(s => assert(isValidUtf8(s), s"[$s] should be valid")) + val invalid = Seq( + "\u0000", // NULL (Cc) + "with an embedded null \u0000 char", // embedded NULL (Cc) + "\u0007", // BEL (Cc) + "\u007f", // DEL (Cc) + "\u009f", // C1 control (Cc) + "\u200b", // zero-width space (Cf) + "\u200e", // left-to-right mark (Cf) + "\ufeff", // zero-width no-break space / BOM (Cf) + "\ue000", // private use (Co) + "\u0378", // unassigned (Cn) + "\ud800", // lone surrogate (Cs) + ) + invalid.foreach(s => assert(!isValidUtf8(s), s"[$s] should be invalid")) + } + test("encode/decode with rgb codec") { val color = Color(47.toByte, 255.toByte, 142.toByte) val bin = rgb.encode(color).require diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala index 1296c6a7c0..dfd5649741 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/wire/protocol/LightningMessageCodecsSpec.scala @@ -210,6 +210,67 @@ class LightningMessageCodecsSpec extends AnyFunSuite { assert(bin == bin2) } + test("decode node_announcements with invalid alias or non-minimally-encoded features") { + // These messages decode successfully (so we don't disconnect the peer that relayed them), but they must be ignored + // by the router: the alias contains control characters and/or the features are not minimally-encoded. + // The 64-bytes signature is set to 0 as it is irrelevant here (it is not checked by the codec). + + // features are 0x0080 (leading zero byte), alias is "ln-node". + val nonMinimalFeatures = + hex"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" ++ // signature + hex"00020080" ++ // features (0x0080 with length prefix) + hex"5acdf507" ++ // timestamp + hex"02d2eabbbacc7c25bbd73b39e65d28237705f7bde76f557e94fb41cb18a9ec0084" ++ // node_id + hex"112211" ++ // color + hex"6c6e2d6e6f646500000000000000000000000000000000000000000000000000" ++ // alias (ln-node) + hex"0000" // addrlen + val ann1 = nodeAnnouncementCodec.decode(nonMinimalFeatures.bits).require.value + assert(ann1.alias == "ln-node") + assert(CommonCodecs.isValidUtf8(ann1.alias)) + assert(ann1.features.toByteVector == hex"80") + assert(!ann1.features.isMinimallyEncoded) + + // features are 0x80 (minimally-encoded), alias is "alice" followed by a BEL control character (0x07). + val invalidAlias = + hex"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" ++ // signature + hex"000180" ++ // features (0x80 with length prefix) + hex"5acdf507" ++ // timestamp + hex"02d2eabbbacc7c25bbd73b39e65d28237705f7bde76f557e94fb41cb18a9ec0084" ++ // node_id + hex"112211" ++ // color + hex"616c696365070000000000000000000000000000000000000000000000000000" ++ // alias (ln-node) + hex"0000" // addrlen + val ann2 = nodeAnnouncementCodec.decode(invalidAlias.bits).require.value + assert(ann2.alias == "alice\u0007") + assert(ann2.features.isMinimallyEncoded) + assert(!CommonCodecs.isValidUtf8(ann2.alias)) + } + + test("node_announcement invalid alias (official test vectors)") { + // These test vectors are validly signed by the following fixed node key, but must be ignored by readers because + // they violate the encoding rules of https://github.com/lightning/bolts/pull/1341. + val nodeKey = PrivateKey(hex"1111111111111111111111111111111111111111111111111111111111111111") + val timestamp = TimestampSecond(1_740_000_000L) + val color = Color(1, 2, 3) + val invalidAliases = Map( + // NULL (Cc) + "lightning\u0000rocks" -> hex"01013a3f3d28c538ae96c5c032e534fedea3e452261ebaea82952456c2a9f7477e7151575ce7e36f2f5b38bb27210c8fbdc72dcd92a99b3df47536ad61d8699cbb1c000067b64b00034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa0102036c696768746e696e6700726f636b7300000000000000000000000000000000000000", + // DEL (Cc) + "lightning\u007frocks" -> hex"01011ac8fe92da793fe0b1472616b6cb4346e746fe72d68c1865f79a5bc6ad371e824ce9c91b83b23a5a65eb6c6e07cd4bd84bec6892958d969559b6059424ac2894000067b64b00034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa0102036c696768746e696e677f726f636b7300000000000000000000000000000000000000", + // zero-width space (Cf) + "lightning\u200brocks" -> hex"0101eb387795f9b453da150cd16b683595ecacc94f6b41d74e1908eef1b8782a6e606e8b9e073ba4fc7eccef5d450faa52e99582db9c620df3fa6f33598220a3b886000067b64b00034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa0102036c696768746e696e67e2808b726f636b730000000000000000000000000000000000", + // private use (Co) + "lightning\ue000rocks" -> hex"0101f3575e38f455655b2e54ea4df8b1745c13938db744d6607fc142cf5be2ce619326ffa6a3f48ac764c7d3d20055fc855f515b77b74c26451862732a8a1f39da1f000067b64b00034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa0102036c696768746e696e67ee8080726f636b730000000000000000000000000000000000", + ) + invalidAliases.foreach { case (alias, encoded) => + val annInvalidAlias = Announcements.makeNodeAnnouncement(nodeKey, alias, color, Nil, Features.empty, timestamp) + assert(Announcements.checkSig(annInvalidAlias)) // the signature is valid + assert(!CommonCodecs.isValidUtf8(annInvalidAlias.alias)) // but the alias contains a control character + assert(lightningMessageCodec.encode(annInvalidAlias).require.bytes == encoded) + val decoded = lightningMessageCodec.decode(encoded.bits).require.value.asInstanceOf[NodeAnnouncement] + assert(!Announcements.checkSig(decoded) || !CommonCodecs.isValidUtf8(decoded.alias)) + } + } + test("encode/decode interactive-tx messages") { val channelId1 = ByteVector32(hex"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") val channelId2 = ByteVector32(hex"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")