From 270ad264ee459561872d7e7647500292fe3d4cd6 Mon Sep 17 00:00:00 2001 From: t-bast Date: Mon, 29 Nov 2021 16:53:40 +0100 Subject: [PATCH] Restrict payment metadata to 128 bytes We can't let unbounded payment metadata be transmitted over the network, otherwise we may not have enough space available for other onion fields (especially for trampoline and route blinding). --- .../eclair/payment/send/PaymentError.scala | 2 ++ .../payment/send/PaymentInitiator.scala | 28 +++++++++++-------- .../eclair/payment/PaymentInitiatorSpec.scala | 14 ++++++++++ 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentError.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentError.scala index 921a8c5bb5..9603b1d911 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentError.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentError.scala @@ -28,6 +28,8 @@ object PaymentError { case class UnsupportedFeatures(features: Features) extends InvalidInvoice { override def getMessage: String = s"unsupported invoice features: ${features.toByteVector.toHex}" } /** The invoice is missing a payment secret. */ case object PaymentSecretMissing extends InvalidInvoice { override def getMessage: String = "invalid invoice: payment secret is missing" } + /** The invoice contains too much payment metadata. */ + case object PaymentMetadataTooLong extends InvalidInvoice { override def getMessage: String = "invalid invoice: payment metatada must be at most 128 bytes" } // @formatter:on // @formatter:off diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala index e6fb861752..ef57e79199 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/payment/send/PaymentInitiator.scala @@ -52,18 +52,22 @@ class PaymentInitiator(nodeParams: NodeParams, outgoingPaymentFactory: PaymentIn } val paymentCfg = SendPaymentConfig(paymentId, paymentId, r.externalId, r.paymentHash, r.recipientAmount, r.recipientNodeId, Upstream.Local(paymentId), Some(r.paymentRequest), storeInDb = true, publishEvent = true, recordPathFindingMetrics = true, Nil) val finalExpiry = r.finalExpiry(nodeParams.currentBlockHeight) - r.paymentRequest.paymentSecret match { - case _ if !r.paymentRequest.features.areSupported(nodeParams) => - sender() ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(r.recipientAmount, Nil, UnsupportedFeatures(r.paymentRequest.features.features)) :: Nil) - case None => - sender() ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(r.recipientAmount, Nil, PaymentSecretMissing) :: Nil) - case Some(paymentSecret) if r.paymentRequest.features.allowMultiPart && nodeParams.features.hasFeature(BasicMultiPartPayment) => - val fsm = outgoingPaymentFactory.spawnOutgoingMultiPartPayment(context, paymentCfg) - fsm ! SendMultiPartPayment(sender(), paymentSecret, r.recipientNodeId, r.recipientAmount, finalExpiry, r.maxAttempts, r.paymentRequest.paymentMetadata, r.assistedRoutes, r.routeParams, userCustomTlvs = r.userCustomTlvs) - case Some(paymentSecret) => - val finalPayload = PaymentOnion.createSinglePartPayload(r.recipientAmount, finalExpiry, paymentSecret, r.paymentRequest.paymentMetadata, r.userCustomTlvs) - val fsm = outgoingPaymentFactory.spawnOutgoingPayment(context, paymentCfg) - fsm ! PaymentLifecycle.SendPaymentToNode(sender(), r.recipientNodeId, finalPayload, r.maxAttempts, r.assistedRoutes, r.routeParams) + if (!r.paymentRequest.features.areSupported(nodeParams)) { + sender() ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(r.recipientAmount, Nil, UnsupportedFeatures(r.paymentRequest.features.features)) :: Nil) + } else if (r.paymentRequest.paymentMetadata.exists(m => m.length > 128)) { + sender() ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(r.recipientAmount, Nil, PaymentMetadataTooLong) :: Nil) + } else { + r.paymentRequest.paymentSecret match { + case None => + sender() ! PaymentFailed(paymentId, r.paymentHash, LocalFailure(r.recipientAmount, Nil, PaymentSecretMissing) :: Nil) + case Some(paymentSecret) if r.paymentRequest.features.allowMultiPart && nodeParams.features.hasFeature(BasicMultiPartPayment) => + val fsm = outgoingPaymentFactory.spawnOutgoingMultiPartPayment(context, paymentCfg) + fsm ! SendMultiPartPayment(sender(), paymentSecret, r.recipientNodeId, r.recipientAmount, finalExpiry, r.maxAttempts, r.paymentRequest.paymentMetadata, r.assistedRoutes, r.routeParams, userCustomTlvs = r.userCustomTlvs) + case Some(paymentSecret) => + val finalPayload = PaymentOnion.createSinglePartPayload(r.recipientAmount, finalExpiry, paymentSecret, r.paymentRequest.paymentMetadata, r.userCustomTlvs) + val fsm = outgoingPaymentFactory.spawnOutgoingPayment(context, paymentCfg) + fsm ! PaymentLifecycle.SendPaymentToNode(sender(), r.recipientNodeId, finalPayload, r.maxAttempts, r.assistedRoutes, r.routeParams) + } } case r: SendSpontaneousPayment => diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala index 652c9fb103..89f815ea2a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentInitiatorSpec.scala @@ -28,6 +28,7 @@ import fr.acinq.eclair.payment.OutgoingPaymentPacket.Upstream import fr.acinq.eclair.payment.PaymentPacketSpec._ import fr.acinq.eclair.payment.PaymentRequest.{ExtraHop, PaymentRequestFeatures} import fr.acinq.eclair.payment.send.MultiPartPaymentLifecycle.SendMultiPartPayment +import fr.acinq.eclair.payment.send.PaymentError.{PaymentMetadataTooLong, UnsupportedFeatures} import fr.acinq.eclair.payment.send.PaymentInitiator._ import fr.acinq.eclair.payment.send.{PaymentError, PaymentInitiator, PaymentLifecycle} import fr.acinq.eclair.router.RouteNotFound @@ -122,6 +123,19 @@ class PaymentInitiatorSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike val fail = sender.expectMsgType[PaymentFailed] assert(fail.id === id) assert(fail.failures.head.isInstanceOf[LocalFailure]) + assert(fail.failures.head.asInstanceOf[LocalFailure].t === UnsupportedFeatures(pr.features.features)) + } + + test("reject payment with long payment metadata") { f => + import f._ + val longPaymentMetadata = Some(hex"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff") + val pr = PaymentRequest(Block.LivenetGenesisBlock.hash, Some(finalAmount), paymentHash, randomKey(), Left("Some invoice"), CltvExpiryDelta(18), paymentMetadata = longPaymentMetadata, features = PaymentRequestFeatures(Features.VariableLengthOnion.mandatory, Features.PaymentSecret.mandatory, Features.PaymentMetadata.optional)) + sender.send(initiator, SendPaymentToNode(finalAmount, pr, 1, CltvExpiryDelta(42), routeParams = nodeParams.routerConf.pathFindingExperimentConf.getRandomConf().getDefaultRouteParams)) + val id = sender.expectMsgType[UUID] + val fail = sender.expectMsgType[PaymentFailed] + assert(fail.id === id) + assert(fail.failures.head.isInstanceOf[LocalFailure]) + assert(fail.failures.head.asInstanceOf[LocalFailure].t === PaymentMetadataTooLong) } test("forward payment with pre-defined route") { f =>