From d982b5282d6826c7f3fbab1ef49ef52c89af5b43 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 4 Feb 2020 11:32:06 +0100 Subject: [PATCH] Ignore invalid hash length Correctly implement skipping over hashed tags with invalid length. See https://github.com/lightningnetwork/lightning-rfc/pull/736. --- .../acinq/eclair/payment/PaymentRequest.scala | 106 +++++++++++------- .../eclair/payment/PaymentRequestSpec.scala | 82 +++++++++++--- 2 files changed, 133 insertions(+), 55 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala index 929c859f70..d1f2fc1944 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala @@ -158,11 +158,12 @@ object PaymentRequest { sealed trait TaggedField - sealed trait UnknownTaggedField extends TaggedField + sealed trait UnknownTaggedField extends TaggedField { + def data: BitVector + } // @formatter:off case class UnknownTag0(data: BitVector) extends UnknownTaggedField - case class UnknownTag1(data: BitVector) extends UnknownTaggedField case class UnknownTag2(data: BitVector) extends UnknownTaggedField case class UnknownTag4(data: BitVector) extends UnknownTaggedField case class UnknownTag7(data: BitVector) extends UnknownTaggedField @@ -187,19 +188,27 @@ object PaymentRequest { case class UnknownTag31(data: BitVector) extends UnknownTaggedField // @formatter:on + // @formatter:off + sealed trait PaymentHashTag extends TaggedField /** * Payment Hash * * @param hash payment hash */ - case class PaymentHash(hash: ByteVector32) extends TaggedField + case class PaymentHash(hash: ByteVector32) extends PaymentHashTag + case class InvalidPaymentHash(data: BitVector) extends PaymentHashTag with UnknownTaggedField + // @formatter:on + // @formatter:off + sealed trait PaymentSecretTag extends TaggedField /** * Payment secret. This is currently random bytes used to protect against probing from the next-to-last node. * * @param secret payment secret */ - case class PaymentSecret(secret: ByteVector32) extends TaggedField + case class PaymentSecret(secret: ByteVector32) extends PaymentSecretTag + case class InvalidPaymentSecret(data: BitVector) extends PaymentSecretTag with UnknownTaggedField + // @formatter:on /** * Description @@ -208,13 +217,17 @@ object PaymentRequest { */ case class Description(description: String) extends TaggedField + // @formatter:off + sealed trait DescriptionHashTag extends TaggedField /** * Hash * * @param hash hash that will be included in the payment request, and can be checked against the hash of a * long description, an invoice, ... */ - case class DescriptionHash(hash: ByteVector32) extends TaggedField + case class DescriptionHash(hash: ByteVector32) extends DescriptionHashTag + case class InvalidDescriptionHash(data: BitVector) extends DescriptionHashTag with UnknownTaggedField + // @formatter:on /** * Fallback Payment that specifies a fallback payment address to be used if LN payment cannot be processed @@ -377,41 +390,58 @@ object PaymentRequest { val dataLengthCodec: Codec[Long] = uint(10).xmap(_ * 5, s => (s / 5 + (if (s % 5 == 0) 0 else 1)).toInt) - def dataCodec[A](valueCodec: Codec[A]): Codec[A] = paddedVarAlignedBits(dataLengthCodec, valueCodec, multipleForPadding = 5) + def dataCodec[A <: TaggedField](valueCodec: Codec[A]): Codec[A] = paddedVarAlignedBits(dataLengthCodec, valueCodec, multipleForPadding = 5) + + def dataCodecLengthDiscriminated[A <: TaggedField](length: Long, valueCodec: Codec[A]): Codec[A] = paddedVarAlignedBits(provide(length), valueCodec, multipleForPadding = 5) + + val paymentHashCodec: Codec[PaymentHashTag] = discriminatorWithDefault( + discriminated.by(dataLengthCodec) + .typecase(260, dataCodecLengthDiscriminated(260, bytes32.as[PaymentHash])), + bits.as[InvalidPaymentHash].upcast[PaymentHashTag]) + + val paymentSecretCodec: Codec[PaymentSecretTag] = discriminatorWithDefault( + discriminated.by(dataLengthCodec) + .typecase(260, dataCodecLengthDiscriminated(260, bytes32.as[PaymentSecret])), + bits.as[InvalidPaymentSecret].upcast[PaymentSecretTag]) + + val descriptionHashCodec: Codec[DescriptionHashTag] = discriminatorWithDefault( + discriminated.by(dataLengthCodec) + .typecase(260, dataCodecLengthDiscriminated(260, bytes32.as[DescriptionHash])), + bits.as[InvalidDescriptionHash].upcast[DescriptionHashTag]) val taggedFieldCodec: Codec[TaggedField] = discriminated[TaggedField].by(ubyte(5)) - .typecase(0, dataCodec(bits).as[UnknownTag0]) - .typecase(1, dataCodec(bytes32).as[PaymentHash]) - .typecase(2, dataCodec(bits).as[UnknownTag2]) - .typecase(3, dataCodec(listOfN(extraHopsLengthCodec, extraHopCodec)).as[RoutingInfo]) - .typecase(4, dataCodec(bits).as[UnknownTag4]) - .typecase(5, dataCodec(bits).as[Features]) - .typecase(6, dataCodec(bits).as[Expiry]) - .typecase(7, dataCodec(bits).as[UnknownTag7]) - .typecase(8, dataCodec(bits).as[UnknownTag8]) - .typecase(9, dataCodec(ubyte(5) :: alignedBytesCodec(bytes)).as[FallbackAddress]) - .typecase(10, dataCodec(bits).as[UnknownTag10]) - .typecase(11, dataCodec(bits).as[UnknownTag11]) - .typecase(12, dataCodec(bits).as[UnknownTag12]) - .typecase(13, dataCodec(alignedBytesCodec(utf8)).as[Description]) - .typecase(14, dataCodec(bits).as[UnknownTag14]) - .typecase(15, dataCodec(bits).as[UnknownTag15]) - .typecase(16, dataCodec(bytes32).as[PaymentSecret]) - .typecase(17, dataCodec(bits).as[UnknownTag17]) - .typecase(18, dataCodec(bits).as[UnknownTag18]) - .typecase(19, dataCodec(bits).as[UnknownTag19]) - .typecase(20, dataCodec(bits).as[UnknownTag20]) - .typecase(21, dataCodec(bits).as[UnknownTag21]) - .typecase(22, dataCodec(bits).as[UnknownTag22]) - .typecase(23, dataCodec(bytes32).as[DescriptionHash]) - .typecase(24, dataCodec(bits).as[MinFinalCltvExpiry]) - .typecase(25, dataCodec(bits).as[UnknownTag25]) - .typecase(26, dataCodec(bits).as[UnknownTag26]) - .typecase(27, dataCodec(bits).as[UnknownTag27]) - .typecase(28, dataCodec(bits).as[UnknownTag28]) - .typecase(29, dataCodec(bits).as[UnknownTag29]) - .typecase(30, dataCodec(bits).as[UnknownTag30]) - .typecase(31, dataCodec(bits).as[UnknownTag31]) + .typecase(0, dataCodec(bits.as[UnknownTag0])) + .typecase(1, paymentHashCodec) + .typecase(2, dataCodec(bits.as[UnknownTag2])) + .typecase(3, dataCodec(listOfN(extraHopsLengthCodec, extraHopCodec).as[RoutingInfo])) + .typecase(4, dataCodec(bits.as[UnknownTag4])) + .typecase(5, dataCodec(bits.as[Features])) + .typecase(6, dataCodec(bits.as[Expiry])) + .typecase(7, dataCodec(bits.as[UnknownTag7])) + .typecase(8, dataCodec(bits.as[UnknownTag8])) + .typecase(9, dataCodec((ubyte(5) :: alignedBytesCodec(bytes)).as[FallbackAddress])) + .typecase(10, dataCodec(bits.as[UnknownTag10])) + .typecase(11, dataCodec(bits.as[UnknownTag11])) + .typecase(12, dataCodec(bits.as[UnknownTag12])) + .typecase(13, dataCodec(alignedBytesCodec(utf8).as[Description])) + .typecase(14, dataCodec(bits.as[UnknownTag14])) + .typecase(15, dataCodec(bits.as[UnknownTag15])) + .typecase(16, paymentSecretCodec) + .typecase(17, dataCodec(bits.as[UnknownTag17])) + .typecase(18, dataCodec(bits.as[UnknownTag18])) + .typecase(19, dataCodec(bits.as[UnknownTag19])) + .typecase(20, dataCodec(bits.as[UnknownTag20])) + .typecase(21, dataCodec(bits.as[UnknownTag21])) + .typecase(22, dataCodec(bits.as[UnknownTag22])) + .typecase(23, descriptionHashCodec) + .typecase(24, dataCodec(bits.as[MinFinalCltvExpiry])) + .typecase(25, dataCodec(bits.as[UnknownTag25])) + .typecase(26, dataCodec(bits.as[UnknownTag26])) + .typecase(27, dataCodec(bits.as[UnknownTag27])) + .typecase(28, dataCodec(bits.as[UnknownTag28])) + .typecase(29, dataCodec(bits.as[UnknownTag29])) + .typecase(30, dataCodec(bits.as[UnknownTag30])) + .typecase(31, dataCodec(bits.as[UnknownTag31])) def fixedSizeTrailingCodec[A](codec: Codec[A], size: Int): Codec[A] = Codec[A]( (data: A) => codec.encode(data), diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala index 424499d345..6bdd9008d4 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentRequestSpec.scala @@ -218,22 +218,30 @@ class PaymentRequestSpec extends FunSuite { } test("On mainnet, please send $30 for coffee beans to the same peer, which supports features 9, 15 and 99, using secret 0x1111111111111111111111111111111111111111111111111111111111111111") { - val ref = "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqpqsq67gye39hfg3zd8rgc80k32tvy9xk2xunwm5lzexnvpx6fd77en8qaq424dxgt56cag2dpt359k3ssyhetktkpqh24jqnjyw6uqd08sgptq44qu" - val pr = PaymentRequest.read(ref) - assert(pr.prefix === "lnbc") - assert(pr.amount === Some(2500000000L msat)) - assert(pr.paymentHash.bytes === hex"0001020304050607080900010203040506070809000102030405060708090102") - assert(pr.paymentSecret === Some(ByteVector32(hex"1111111111111111111111111111111111111111111111111111111111111111"))) - assert(pr.timestamp === 1496314658L) - assert(pr.nodeId === PublicKey(hex"03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad")) - assert(pr.description === Left("coffee beans")) - assert(pr.fallbackAddress().isEmpty) - assert(pr.features.bitmask === bin"1000000000000000000000000000000000000000000000000000000000000000000000000000000000001000001000000000") - assert(!pr.features.allowMultiPart) - assert(!pr.features.requirePaymentSecret) - assert(!pr.features.allowTrampoline) - assert(pr.features.supported) - assert(PaymentRequest.write(pr.sign(priv)) === ref) + val refs = Seq( + "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqpqsq67gye39hfg3zd8rgc80k32tvy9xk2xunwm5lzexnvpx6fd77en8qaq424dxgt56cag2dpt359k3ssyhetktkpqh24jqnjyw6uqd08sgptq44qu", + // All upper-case + "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqpqsq67gye39hfg3zd8rgc80k32tvy9xk2xunwm5lzexnvpx6fd77en8qaq424dxgt56cag2dpt359k3ssyhetktkpqh24jqnjyw6uqd08sgptq44qu".toUpperCase, + // With ignored fields + "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqpqsq2qrqqqfppnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqppnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqhpnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqhp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqspnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqsp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnp5qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnpkqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq2jxxfsnucm4jf4zwtznpaxphce606fvhvje5x7d4gw7n73994hgs7nteqvenq8a4ml8aqtchv5d9pf7l558889hp4yyrqv6a7zpq9fgpskqhza" + ) + + for (ref <- refs) { + val pr = PaymentRequest.read(ref) + assert(pr.prefix === "lnbc") + assert(pr.amount === Some(2500000000L msat)) + assert(pr.paymentHash.bytes === hex"0001020304050607080900010203040506070809000102030405060708090102") + assert(pr.paymentSecret === Some(ByteVector32(hex"1111111111111111111111111111111111111111111111111111111111111111"))) + assert(pr.timestamp === 1496314658L) + assert(pr.nodeId === PublicKey(hex"03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad")) + assert(pr.description === Left("coffee beans")) + assert(pr.features.bitmask === bin"1000000000000000000000000000000000000000000000000000000000000000000000000000000000001000001000000000") + assert(!pr.features.allowMultiPart) + assert(!pr.features.requirePaymentSecret) + assert(!pr.features.allowTrampoline) + assert(pr.features.supported) + assert(PaymentRequest.write(pr.sign(priv)) === ref.toLowerCase) + } } test("On mainnet, please send $30 for coffee beans to the same peer, which supports features 9, 15, 99 and 100, using secret 0x1111111111111111111111111111111111111111111111111111111111111111") { @@ -272,10 +280,30 @@ class PaymentRequestSpec extends FunSuite { assert(PaymentRequest.write(pr.sign(priv)) === ref) } + test("reject invalid invoices") { + val refs = Seq( + // Bech32 checksum is invalid. + "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdpquwpc4curk03c9wlrswe78q4eyqc7d8d0xqzpuyk0sg5g70me25alkluzd2x62aysf2pyy8edtjeevuv4p2d5p76r4zkmneet7uvyakky2zr4cusd45tftc9c5fh0nnqpnl2jfll544esqchsrnt", + // Malformed bech32 string (no 1). + "pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdpquwpc4curk03c9wlrswe78q4eyqc7d8d0xqzpuyk0sg5g70me25alkluzd2x62aysf2pyy8edtjeevuv4p2d5p76r4zkmneet7uvyakky2zr4cusd45tftc9c5fh0nnqpnl2jfll544esqchsrny", + // Malformed bech32 string (mixed case). + "LNBC2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdpquwpc4curk03c9wlrswe78q4eyqc7d8d0xqzpuyk0sg5g70me25alkluzd2x62aysf2pyy8edtjeevuv4p2d5p76r4zkmneet7uvyakky2zr4cusd45tftc9c5fh0nnqpnl2jfll544esqchsrny", + // Signature is not recoverable. + "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpuaxtrnwngzn3kdzw5hydlzf03qdgm2hdq27cqv3agm2awhz5se903vruatfhq77w3ls4evs3ch9zw97j25emudupq63nyw24cg27h2rspk28uwq", + // String is too short. + "lnbc1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdpl2pkx2ctnv5sxxmmwwd5kgetjypeh2ursdae8g6na6hlh", + // Invalid multiplier. + "lnbc2500x1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpujr6jxr9gq9pv6g46y7d20jfkegkg4gljz2ea2a3m9lmvvr95tq2s0kvu70u3axgelz3kyvtp2ywwt0y8hkx2869zq5dll9nelr83zzqqpgl2zg" + ) + for (ref <- refs) { + assertThrows[Exception](PaymentRequest.read(ref)) + } + } + test("correctly serialize/deserialize variable-length tagged fields") { val number = 123456 - val codec = PaymentRequest.Codecs.dataCodec(scodec.codecs.bits).as[PaymentRequest.Expiry] + val codec = PaymentRequest.Codecs.dataCodec(scodec.codecs.bits.as[PaymentRequest.Expiry]) val field = PaymentRequest.Expiry(number) assert(field.toLong == number) @@ -310,6 +338,26 @@ class PaymentRequestSpec extends FunSuite { val Some(_) = pr1.tags.collectFirst { case u: UnknownTag21 => u } } + test("ignore hash tags with invalid length") { + // Bolt11: A reader: MUST skip over p, h, s or n fields that do NOT have data_lengths of 52, 52, 52 or 53, respectively. + val inputs = Seq( + "ppnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "pp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "hpnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "hp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "spnqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "sp4qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "np5qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq", + "npkqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq" + ) + + for (input <- inputs) { + val data = string2Bits(input) + val decoded = Codecs.taggedFieldCodec.decode(data).require.value + assert(decoded.isInstanceOf[UnknownTaggedField], input) + } + } + test("accept uppercase payment request") { val input = "lntb1500n1pwxx94fpp5q3xzmwuvxpkyhz6pvg3fcfxz0259kgh367qazj62af9rs0pw07dsdpa2fjkzep6yp58garswvaz7tmvd9nksarwd9hxw6n0w4kx2tnrdakj7grfwvs8wcqzysxqr23sjzv0d8794te26xhexuc26eswf9sjpv4t8sma2d9y8dmpgf0qseg8259my8tcs6zte7ex0tz4exm5pjezuxrq9u0vjewa02qhedk9x4gppweupu"