Merge pull request #692 from Routstr/fix/lnurl-amount-and-refund-state

fix: verify LNURL invoice amount and destination before reserving

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(

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:

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

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