From c11b6d30c5aed34f5332ce04ceb795b282d6f4d4 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Mon, 24 Aug 2026 01:47:07 +0200 Subject: [PATCH] fix: verify LNURL invoice amount and destination before reserving raw_send_to_lnurl asked an LNURL service for an invoice of a specific amount and then quoted and melted whatever invoice came back, without ever checking the two agreed. A malicious or compromised payout service could return an invoice for far more than requested and be paid up to the value of the selected proofs. The melt quote is the mint's own reading of the invoice, so it is compared against the requested amount in the wallet unit before the quote checkpoint and before any proof is reserved: a mismatch now leaves no durable state behind. Destinations were equally unguarded. A bech32 LNURL could decode to a plaintext or internal URL, and both fetches followed redirects blindly, so a public https origin could bounce the request onto loopback or link local metadata addresses. Redirects are now followed manually with the scheme and host re-checked at every hop, and the callback URL taken from the payRequest body is checked the same way. Error messages no longer echo service-controlled response bodies into operator logs. The user refund path reserved proofs and then called raw_send_to_lnurl without an amount, which the callee rejected before dispatch. Every such refund failed with the proofs still locked, and the proof rollback that covers a dispatched melt does not reach that stage. send_to_lnurl now hands over the unreserved available proofs and the amount, leaving reservation to happen only after the destination, the invoice amount and the quote have all been accepted. Pre-dispatch rejection therefore unwinds cleanly, while an ambiguous dispatch still raises MeltOutcomeAmbiguousError and keeps the debit for reconciliation. The missing amount was previously caught by a bare assert, which was both unreachable for the payout callers and the wrong failure mode for the refund caller; it is now an explicit validation. --- routstr/payment/lnurl.py | 138 ++++++--- routstr/wallet.py | 6 +- .../unit/test_lnurl_amount_and_destination.py | 293 ++++++++++++++++++ tests/unit/test_lnurl_melt_timeout.py | 8 +- 4 files changed, 406 insertions(+), 39 deletions(-) create mode 100644 tests/unit/test_lnurl_amount_and_destination.py diff --git a/routstr/payment/lnurl.py b/routstr/payment/lnurl.py index 37ecb1cf..c9f48253 100644 --- a/routstr/payment/lnurl.py +++ b/routstr/payment/lnurl.py @@ -1,8 +1,9 @@ from __future__ import annotations +import ipaddress import math from collections.abc import Awaitable, Callable -from typing import TypedDict +from typing import Any, TypedDict import httpx from cashu.core.base import MeltQuoteState @@ -42,6 +43,70 @@ class MeltOutcomeAmbiguousError(LNURLError): """ +_MAX_LNURL_REDIRECTS = 3 +_NON_PUBLIC_HOST_SUFFIXES = (".localhost", ".local", ".internal") + + +def _require_public_https_destination(url: httpx.URL) -> None: + """Reject anything that is not a public HTTPS endpoint. + + LNURL destinations and their redirect targets are attacker-influenced, so + every hop has to be re-checked: a single ``https://`` origin says nothing + about where a 302 points. + """ + if url.scheme != "https": + raise LNURLError("LNURL destination must be an HTTPS URL") + + host = (url.host or "").rstrip(".").lower() + if not host: + raise LNURLError("LNURL destination has no host") + + try: + address = ipaddress.ip_address(host) + except ValueError: + if host == "localhost" or host.endswith(_NON_PUBLIC_HOST_SUFFIXES): + raise LNURLError("LNURL destination is not a public host") from None + return + + if not address.is_global: + raise LNURLError("LNURL destination is not a public host") + + +async def _fetch_lnurl_json( + url: str, params: dict[str, int] | None = None +) -> dict[str, Any]: + """GET an LNURL endpoint, validating the destination at every redirect. + + Response bodies are never echoed: an LNURL service is untrusted, and its + payload would otherwise reach operator logs through raised errors. + """ + try: + target = httpx.URL(url, params=params) if params else httpx.URL(url) + except httpx.InvalidURL as e: + raise LNURLError("LNURL destination is not a usable URL") from e + _require_public_https_destination(target) + + async with httpx.AsyncClient() as client: + for _ in range(_MAX_LNURL_REDIRECTS + 1): + response = await client.get(target, follow_redirects=False, timeout=10) + if not response.is_redirect: + break + target = target.join(response.headers.get("location", "")) + _require_public_https_destination(target) + else: + raise LNURLError("LNURL destination exceeded the redirect limit") + response.raise_for_status() + + try: + data = response.json() + except ValueError as e: + raise LNURLError("LNURL response was not valid JSON") from e + + if not isinstance(data, dict): + raise LNURLError("LNURL response was not a JSON object") + return data + + async def decode_lnurl(lnurl: str) -> str: """Decode LNURL to get the actual URL. @@ -111,26 +176,29 @@ async def get_lnurl_data(lnurl: str) -> LNURLData: httpx.HTTPError: If the HTTP request fails """ url = await decode_lnurl(lnurl) - - async with httpx.AsyncClient() as client: - response = await client.get(url, follow_redirects=True, timeout=10) - response.raise_for_status() - - lnurl_data = response.json() + lnurl_data = await _fetch_lnurl_json(url) # Validate payRequest data if lnurl_data.get("tag") != "payRequest": - raise LNURLError( - f"Invalid LNURL tag: expected 'payRequest', got '{lnurl_data.get('tag')}'" - ) + raise LNURLError("Invalid LNURL tag: expected 'payRequest'") - if not isinstance(lnurl_data.get("callback"), str): + callback_url = lnurl_data.get("callback") + if not isinstance(callback_url, str): raise LNURLError("Invalid LNURL payRequest: missing callback URL") + try: + _require_public_https_destination(httpx.URL(callback_url)) + except httpx.InvalidURL as e: + raise LNURLError("Invalid LNURL callback URL") from e + + min_sendable = lnurl_data.get("minSendable", 1000) # Default 1 sat + max_sendable = lnurl_data.get("maxSendable", 1000000000) # Default 1000 BTC + if not isinstance(min_sendable, int) or not isinstance(max_sendable, int): + raise LNURLError("Invalid LNURL payRequest: non-integer sendable limits") return LNURLData( - callback_url=lnurl_data["callback"], - min_sendable=lnurl_data.get("minSendable", 1000), # Default 1 sat - max_sendable=lnurl_data.get("maxSendable", 1000000000), # Default 1000 BTC + callback_url=callback_url, + min_sendable=min_sendable, + max_sendable=max_sendable, ) @@ -150,22 +218,10 @@ async def get_lnurl_invoice( LNURLError: If the response is invalid httpx.HTTPError: If the HTTP request fails """ - async with httpx.AsyncClient() as client: - response = await client.get( - callback_url, - params={"amount": amount_msat}, - follow_redirects=True, - timeout=10, - ) - response.raise_for_status() + invoice_data = await _fetch_lnurl_json(callback_url, params={"amount": amount_msat}) - invoice_data = response.json() - - if "pr" not in invoice_data: - # Check if there's an error in the response - if "reason" in invoice_data: - raise LNURLError(f"LNURL error: {invoice_data['reason']}") - raise LNURLError(f"Invalid LNURL invoice response: {invoice_data}") + if not isinstance(invoice_data.get("pr"), str): + raise LNURLError("LNURL callback returned no invoice") return invoice_data["pr"], invoice_data @@ -201,12 +257,11 @@ async def raw_send_to_lnurl( # Send USD to Lightning Address paid = await wallet.send_to_lnurl("user@getalby.com", 50, unit="usd") """ - total_balance = sum(proof.amount for proof in proofs) - if amount and total_balance < amount: + if not isinstance(amount, int) or isinstance(amount, bool) or amount <= 0: + raise ValueError("A positive integer amount is required to send to an LNURL.") + if sum(proof.amount for proof in proofs) < amount: raise ValueError("Amount to send is higher than available proofs.") - else: - assert isinstance(amount, int) - total_balance = amount + total_balance = amount lnurl_data = await get_lnurl_data(lnurl) if unit == "sat": @@ -240,11 +295,22 @@ async def raw_send_to_lnurl( mint_url=str(wallet.url), ) + # The invoice comes from the LNURL service, so its amount is untrusted. The + # melt quote is the mint's own reading of it, and it must match what we + # asked to send. Checked before the checkpoint and before reserving, so a + # mismatch leaves no durable state and no locked proofs behind. + quoted_amount = int(melt_quote_resp.amount) + expected_amount = final_amount // 1000 if unit == "sat" else final_amount + if quoted_amount != expected_amount: + raise LNURLError( + f"LNURL invoice amount does not match the requested amount " + f"(quoted {quoted_amount} {unit}, expected {expected_amount} {unit})" + ) + if on_melt_quote is not None: await on_melt_quote(melt_quote_resp.quote) - if amount: - proofs, _ = await wallet.select_to_send(proofs, amount, set_reserved=True) + proofs, _ = await wallet.select_to_send(proofs, amount, set_reserved=True) try: melt_response = await run_mint_operation( diff --git a/routstr/wallet.py b/routstr/wallet.py index f83ff8a6..c6cb1238 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -2622,8 +2622,10 @@ async def send_to_lnurl(amount: int, unit: str, mint: str, address: str) -> int: mint = await find_trusted_mint_with_funds(amount, unit, mint, force_reload=True) wallet = await get_wallet(mint, unit) available = get_proofs_per_mint_and_unit(wallet, mint, unit, not_reserved=True) - proofs, _ = await wallet.select_to_send(available, amount, set_reserved=True) - return await raw_send_to_lnurl(wallet, proofs, address, unit) + # Hand over unreserved proofs: raw_send_to_lnurl reserves only once the + # destination, the invoice amount and the melt quote have all been + # accepted, so a rejected refund cannot strand locked proofs. + return await raw_send_to_lnurl(wallet, available, address, unit, amount=amount) # class Payment: diff --git a/tests/unit/test_lnurl_amount_and_destination.py b/tests/unit/test_lnurl_amount_and_destination.py new file mode 100644 index 00000000..5ba459d9 --- /dev/null +++ b/tests/unit/test_lnurl_amount_and_destination.py @@ -0,0 +1,293 @@ +"""LNURL payments must verify the invoice amount and the destination. + +Findings 4 and 5 ship together: the amount check is what makes it safe to +hand an LNURL a set of unreserved proofs, and reserving only after that check +is what stops a pre-dispatch failure from stranding proofs. +""" + +from collections.abc import Callable +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest +from cashu.core.base import MeltQuoteState + +from routstr.payment import lnurl as lnurl_module +from routstr.payment.lnurl import ( + LNURLError, + get_lnurl_data, + get_lnurl_invoice, + raw_send_to_lnurl, +) + +LNURL_DATA = { + "callback_url": "https://ln.tld/cb", + "min_sendable": 1_000, + "max_sendable": 100_000_000, +} + +# 1000 sat minus the 11 sat estimated fee reserve. +EXPECTED_QUOTE_SAT = 989 + + +def _wallet( + quote_amount: int = EXPECTED_QUOTE_SAT, +) -> tuple[MagicMock, list[MagicMock]]: + proofs = [MagicMock(amount=1000)] + wallet = MagicMock(url="https://mint.test") + wallet.melt_quote = AsyncMock( + return_value=MagicMock(fee_reserve=1, quote="q", amount=quote_amount) + ) + wallet.select_to_send = AsyncMock(return_value=(proofs, None)) + wallet.melt = AsyncMock(return_value=MagicMock(state=MeltQuoteState.paid)) + wallet.set_reserved_for_send = AsyncMock() + return wallet, proofs + + +def _lnurl_patches() -> tuple[Any, Any]: + return ( + patch( + "routstr.payment.lnurl.get_lnurl_data", + AsyncMock(return_value=LNURL_DATA), + ), + patch( + "routstr.payment.lnurl.get_lnurl_invoice", + AsyncMock(return_value=("lnbc1...", {})), + ), + ) + + +def _mock_client(handler: Callable[[httpx.Request], httpx.Response]) -> Any: + real_client = httpx.AsyncClient + + def factory(*_args: object, **_kwargs: object) -> httpx.AsyncClient: + return real_client(transport=httpx.MockTransport(handler)) + + return patch.object(lnurl_module.httpx, "AsyncClient", factory) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "quote_amount", [EXPECTED_QUOTE_SAT + 1, EXPECTED_QUOTE_SAT * 5] +) +async def test_raw_send_to_lnurl_rejects_oversized_invoice(quote_amount: int) -> None: + wallet, proofs = _wallet(quote_amount) + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch, pytest.raises(LNURLError, match="invoice amount"): + await raw_send_to_lnurl(wallet, proofs, "owner@ln.tld", "sat", amount=1000) + + wallet.select_to_send.assert_not_awaited() + wallet.melt.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_raw_send_to_lnurl_rejects_undersized_invoice() -> None: + wallet, proofs = _wallet(EXPECTED_QUOTE_SAT - 1) + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch, pytest.raises(LNURLError, match="invoice amount"): + await raw_send_to_lnurl(wallet, proofs, "owner@ln.tld", "sat", amount=1000) + + wallet.select_to_send.assert_not_awaited() + wallet.melt.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_raw_send_to_lnurl_rejects_invoice_before_quote_checkpoint() -> None: + wallet, proofs = _wallet(EXPECTED_QUOTE_SAT * 2) + checkpoint = AsyncMock() + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch, pytest.raises(LNURLError): + await raw_send_to_lnurl( + wallet, + proofs, + "owner@ln.tld", + "sat", + amount=1000, + on_melt_quote=checkpoint, + ) + + checkpoint.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_raw_send_to_lnurl_accepts_exact_invoice() -> None: + wallet, proofs = _wallet() + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch: + paid = await raw_send_to_lnurl( + wallet, proofs, "owner@ln.tld", "sat", amount=1000 + ) + + assert paid == EXPECTED_QUOTE_SAT * 1000 + wallet.select_to_send.assert_awaited_once() + wallet.melt.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_raw_send_to_lnurl_msat_unit_compares_in_wallet_unit() -> None: + # 1_000_000 msat minus an 11 sat fee reserve leaves 989_000 msat. + wallet, proofs = _wallet(989_000) + proofs[0].amount = 1_000_000 + wallet.select_to_send = AsyncMock(return_value=(proofs, None)) + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch: + paid = await raw_send_to_lnurl( + wallet, proofs, "owner@ln.tld", "msat", amount=1_000_000 + ) + + assert paid == 989_000 + + +@pytest.mark.asyncio +async def test_raw_send_to_lnurl_requires_an_explicit_amount() -> None: + wallet, proofs = _wallet() + data_patch, invoice_patch = _lnurl_patches() + + with data_patch, invoice_patch, pytest.raises(ValueError, match="amount"): + await raw_send_to_lnurl(wallet, proofs, "owner@ln.tld", "sat") + + wallet.select_to_send.assert_not_awaited() + wallet.melt.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "address", + [ + "owner@127.0.0.1", + "owner@localhost", + "owner@10.0.0.5", + "owner@[::1]", + "http://ln.tld/lnurlp/owner", + ], +) +async def test_get_lnurl_data_rejects_non_public_destination(address: str) -> None: + requested: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + requested.append(str(request.url)) + return httpx.Response( + 200, json={"tag": "payRequest", "callback": "https://x/y"} + ) + + with _mock_client(handler), pytest.raises(LNURLError): + await get_lnurl_data(address) + + assert requested == [] + + +@pytest.mark.asyncio +async def test_get_lnurl_data_rejects_redirect_to_private_host() -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.host == "ln.tld": + return httpx.Response( + 302, headers={"location": "https://169.254.169.254/latest/meta-data"} + ) + return httpx.Response( + 200, json={"tag": "payRequest", "callback": "https://x/y"} + ) + + with _mock_client(handler), pytest.raises(LNURLError, match="destination"): + await get_lnurl_data("owner@ln.tld") + + +@pytest.mark.asyncio +async def test_get_lnurl_data_rejects_downgrade_redirect() -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.scheme == "https": + return httpx.Response(302, headers={"location": "http://ln.tld/plain"}) + return httpx.Response( + 200, json={"tag": "payRequest", "callback": "https://x/y"} + ) + + with _mock_client(handler), pytest.raises(LNURLError, match="destination"): + await get_lnurl_data("owner@ln.tld") + + +@pytest.mark.asyncio +async def test_get_lnurl_data_rejects_private_callback_url() -> None: + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json={"tag": "payRequest", "callback": "http://127.0.0.1:8000/cb"} + ) + + with _mock_client(handler), pytest.raises(LNURLError, match="destination"): + await get_lnurl_data("owner@ln.tld") + + +@pytest.mark.asyncio +async def test_get_lnurl_data_error_does_not_leak_response_body() -> None: + secret = "SUPERSECRETBODYMARKER" + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"tag": secret, "callback": secret}) + + with _mock_client(handler), pytest.raises(LNURLError) as excinfo: + await get_lnurl_data("owner@ln.tld") + + assert secret not in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_get_lnurl_invoice_error_does_not_leak_response_body() -> None: + secret = "SUPERSECRETBODYMARKER" + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"reason": secret, "internal": secret}) + + with _mock_client(handler), pytest.raises(LNURLError) as excinfo: + await get_lnurl_invoice("https://ln.tld/cb", 1000) + + assert secret not in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_get_lnurl_invoice_rejects_redirect_to_private_host() -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.host == "ln.tld": + return httpx.Response(302, headers={"location": "https://192.168.1.1/cb"}) + return httpx.Response(200, json={"pr": "lnbc1..."}) + + with _mock_client(handler), pytest.raises(LNURLError, match="destination"): + await get_lnurl_invoice("https://ln.tld/cb", 1000) + + +@pytest.mark.asyncio +async def test_send_to_lnurl_does_not_reserve_before_lnurl_validation() -> None: + from routstr import wallet as wallet_module + + wallet, proofs = _wallet() + + with ( + patch.object( + wallet_module, + "find_trusted_mint_with_funds", + AsyncMock(return_value="https://mint.test"), + ), + patch.object(wallet_module, "get_wallet", AsyncMock(return_value=wallet)), + patch.object( + wallet_module, + "get_proofs_per_mint_and_unit", + MagicMock(return_value=proofs), + ), + patch.object( + wallet_module, + "raw_send_to_lnurl", + AsyncMock(side_effect=LNURLError("destination rejected")), + ) as raw_send, + pytest.raises(LNURLError), + ): + await wallet_module.send_to_lnurl( + 1000, "sat", "https://mint.test", "owner@ln.tld" + ) + + wallet.select_to_send.assert_not_awaited() + assert raw_send.await_args is not None + assert raw_send.await_args.args[1] is proofs + assert raw_send.await_args.kwargs["amount"] == 1000 diff --git a/tests/unit/test_lnurl_melt_timeout.py b/tests/unit/test_lnurl_melt_timeout.py index a370a7a8..dc35ab4c 100644 --- a/tests/unit/test_lnurl_melt_timeout.py +++ b/tests/unit/test_lnurl_melt_timeout.py @@ -22,10 +22,16 @@ LNURL_DATA = { } +# 1000 sat minus the 11 sat estimated fee reserve. +QUOTE_AMOUNT_SAT = 989 + + def _wallet() -> tuple[MagicMock, list[MagicMock]]: proofs = [MagicMock(amount=1000)] wallet = MagicMock(url="https://mint.test") - wallet.melt_quote = AsyncMock(return_value=MagicMock(fee_reserve=1, quote="q")) + wallet.melt_quote = AsyncMock( + return_value=MagicMock(fee_reserve=1, quote="q", amount=QUOTE_AMOUNT_SAT) + ) wallet.select_to_send = AsyncMock(return_value=(proofs, None)) return wallet, proofs