From 5fd8be64fde9d1b34c36fa3f651eebf85f87ebdc Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sun, 19 Jul 2026 18:59:18 -0400 Subject: [PATCH] fix(podcast): clamp V4V fee splits so a feed cannot multiply payments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `computeShares` paid each fee recipient `totalMilliSats * split / 100` with no upper bound on `split`, and `split` comes verbatim from a kind-30054 episode event that anyone can publish (`ValueTag.parse` is a bare `fromJson` with no validation). A single `fee:true, split:1000` recipient was therefore paid TEN TIMES the amount the user chose. The `remainder` clamp looked like a safety net but only zeroed the honest recipients; it never touched the fee recipients themselves. Proven by test before fixing: `split:1000` pays 10x, and two fee recipients at 60% each pay 1,200,000 millisats for a 1,000,000 zap. This was reachable in the worst possible place. Streaming V4V pays every minute, automatically, so a modest multiplier stays under a typical NWC budget and simply runs — and the on-screen running total tracks the INTENDED amount, so it reads "100 sats" while 1,000 left the wallet, while the streaming error handler suppresses the toast. The ordinary zap button reroutes through the same path for any note carrying a value block, so it was not limited to the streaming toggle. Fees are a percentage off the top, so each is now clamped to 100% and the cumulative total to the remaining budget: the payout can never exceed what the user chose. The existing test asserted the right invariant (`sum <= total`) but only ever ran it on well-formed input, which is why this survived. Co-Authored-By: Claude Opus 4.8 --- .../quartz/podcasts/PodcastValue.kt | 12 ++++++- .../quartz/podcasts/PodcastValueShareTest.kt | 33 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValue.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValue.kt index 5f5cf7c270..77cc66ad95 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValue.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValue.kt @@ -66,11 +66,18 @@ class PodcastValue( val active = recipients.filter { it.split > 0 && !it.address.isNullOrBlank() } if (active.isEmpty()) return emptyList() + // The value block is attacker-controlled: it comes verbatim from an episode event that anyone + // can publish, and `split` is an unvalidated Int. A fee is a PERCENTAGE off the top, so it is + // clamped to 100 individually and to whatever budget is left collectively — otherwise a single + // `fee:true, split:1000` recipient would be paid 10x the amount the user actually chose, every + // minute, while the on-screen running total still showed the intended figure. var feeTotalMillis = 0L val feeAmounts = HashMap() for (recipient in active) { if (recipient.fee == true) { - val millis = totalMilliSats * recipient.split / 100 + val percent = recipient.split.coerceAtMost(MAX_FEE_PERCENT) + val budgetLeft = totalMilliSats - feeTotalMillis + val millis = (totalMilliSats * percent / 100).coerceAtMost(budgetLeft) if (millis > 0) { feeAmounts[recipient] = millis feeTotalMillis += millis @@ -97,6 +104,9 @@ class PodcastValue( } companion object { + /** A fee recipient's split is a percentage off the top; anything above 100 is malformed. */ + const val MAX_FEE_PERCENT = 100 + /** * TLV record type for the Podcasting-2.0 keysend metadata blob (the "boostagram"), a JSON * object carrying podcast/episode/app/value context. Registered value, used by the whole diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValueShareTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValueShareTest.kt index 391fac4952..c949b0fa1a 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValueShareTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/podcasts/PodcastValueShareTest.kt @@ -31,6 +31,39 @@ class PodcastValueShareTest { fee: Boolean? = null, ) = PodcastValueRecipient(name = name, type = PodcastValue.TYPE_NODE, address = "node-$name", split = split, fee = fee) + @Test + fun `a hostile fee split can never pay out more than the total`() { + // The value block comes verbatim from a kind-30054 episode event, which anyone can publish, + // and `split` is an unvalidated Int. A fee recipient is paid `total * split / 100`, so a + // split of 1000 would bill the user 10x what they chose — every minute, for a streaming + // payment whose on-screen running total shows only the intended amount. + val value = + PodcastValue( + recipients = listOf(node("attacker", 1000, fee = true), node("host", 100)), + ) + + val shares = value.computeShares(1_000_000L) + val paid = shares.sumOf { it.amountMilliSats } + + assertTrue(paid <= 1_000_000L, "paid $paid millisats for a 1,000,000 millisat zap") + } + + @Test + fun `fee splits summing over 100 percent cannot exceed the total either`() { + val value = + PodcastValue( + recipients = + listOf( + node("feeA", 60, fee = true), + node("feeB", 60, fee = true), + node("host", 100), + ), + ) + + val paid = value.computeShares(1_000_000L).sumOf { it.amountMilliSats } + assertTrue(paid <= 1_000_000L, "paid $paid millisats for a 1,000,000 millisat zap") + } + @Test fun `weighted split with no fees divides by relative weight`() { val value =