fix: validate and restrict LNURL request targets - #3597
Draft
reneaaron wants to merge 3 commits into
Draft
Conversation
LNURL endpoints are supplied by the visited website but fetched from a privileged context with broad host permissions. Restrict those fetches: - require https (http only for .onion), and reject loopback, private, link-local, CGNAT and cloud-metadata hosts, for the LNURL detail request, the lnurl-auth login request, and the pay/withdraw/channel callbacks - refuse to follow redirects on these requests so a permitted host cannot bounce them to a denied one - return a fixed message when a detail request fails instead of relaying the upstream response text Adds a shared lnurlValidation helper with unit coverage, and bridges Node's global fetch into the jsdom test environment so the fetch adapter (and msw) work under test.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- expand IPv6 literals before classifying them, so IPv4-mapped (::ffff:127.0.0.1), IPv4-compatible and NAT64 (64:ff9b::) forms are recognised after the URL parser re-serialises them as hex, and add site-local fec0::/10 - assert the blocked hosts through the URL parser in the tests, since that is the form the validator actually receives - keep the LNURL service's own error text (LUD-06 status: "ERROR") now that the endpoint host is validated, and drop the message that claimed a lightning address was invalid when the server was merely unreachable - take the abort primitives from the same realm as fetch in the test environment
The restrictions are there because a website can hand the extension an endpoint that is then requested from a privileged context. An LNURL the user pasted or scanned themselves is a target they chose, and may well be a self-hosted service on a local network over http. - getDetails takes a userInitiated flag; Send and LNURLRedeem set it, so those keep working against a local service and may follow redirects. The website-driven paths (webln.lnurl, PublisherLnData) are unchanged. - callbacks are allowed on the origin of the LNURL that produced them, and otherwise have to satisfy the usual restrictions, so a self-hosted service's own callback works while a cross-host callback still cannot point at a private address.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the changes you have made in this PR
LNURL endpoints supplied by a website (via
window.webln.lnurl, or anlnurlfound on the page) are requested from the extension's privileged background/prompt context, which holds broad host permissions. This restricts which targets those requests may reach.src/common/lib/lnurlValidation.ts:assertAllowedLnurlUrl(url)— requireshttps:(allowshttp:only for.onion) and rejects loopback, private (RFC1918), CGNAT, link-local, cloud-metadata, multicast and reserved targets. IPv6 literals are expanded before classification, so IPv4-mapped (::ffff:127.0.0.1), IPv4-compatible and NAT64 (64:ff9b::) forms are recognised after the URL parser re-serialises them as hex;fe80::/10,fc00::/7andfec0::/10are covered.assertAllowedCallbackUrl(callback, lnurlUrl)— a callback may stay on the origin of the LNURL that produced it; otherwise it must satisfy the same restrictions. Cross-host callbacks (common for lightning addresses) keep working.lnurlGet(url, config, opts)— an axios GET that validates the target and, for website-supplied LNURLs, does not follow redirects.common/lib/lnurl.ts), the lnurl-auth login request (actions/lnurl/auth.ts) and the pay / withdraw / channel callbacks.getDetailstakes auserInitiatedflag.SendandLNURLRedeemset it, so an LNURL the user pasted or scanned — which may be a self-hosted service on a local network over http — keeps working and may follow redirects. The website-driven paths are unaffected.status: "ERROR"reason is still shown, since the endpoint host has been validated.Behaviour changes worth noting
.onion) and cannot point at a local/private address. User-pasted LNURLs are unrestricted.manualredirect is opaque, with no readableLocation), so they are refused rather than re-checked. Worth a second opinion: if a provider redirects/.well-known/lnurlp/<user>to another host, paying that address from a website would now fail. User-initiated payments still follow redirects.Tests
src/common/lib/__tests__/lnurlValidation.test.ts— host classification, plus URL-level assertions that go through the WHATWG parser (the form the validator actually receives), and callback origin handling.fetch/Request/Responseand the matching abort primitives from one realm, so the axios fetch adapter (and msw) work under test.yarn lint,yarn tsc:compileandyarn test:unitall pass (76 suites, 209 tests, 2 pre-existing skips).