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).
ACINQ · Nov 29, 2021 · 270ad26 · 270ad26
1 parent e956923
commit 270ad26
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 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
@@ -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
@@ -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
@@ -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 =>