From f52b78307553a6ef7938f0901b38c40d403e80ea Mon Sep 17 00:00:00 2001
From: 2ro <17595647+2ro@users.noreply.github.com>
Date: Mon, 6 Jul 2026 07:52:40 -0400
Subject: [PATCH] goblin: bind the login callback host to the approved domain

The login URI parser checked the callback scheme but not that its host belongs
to the displayed domain, so a site could show d=magick.market while pointing cb
at its own host and harvest a signed login event for a domain it does not
control. Apply the same domain binding the authorize URI already enforces:
share authuri's domain_bound as pub(crate) and reject a login URI whose callback
host is neither the domain nor a label-boundary subdomain of it. The
http://localhost dev callback stays exempt. Adds binding tests.
---
 src/nostr/authuri.rs  |  5 ++++-
 src/nostr/loginuri.rs | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/nostr/authuri.rs b/src/nostr/authuri.rs
index de9f40bf..f46f468b 100644
--- a/src/nostr/authuri.rs
+++ b/src/nostr/authuri.rs
@@ -262,7 +262,10 @@ fn is_localhost_authority(rest: &str) -> bool {
 /// server legitimately serves any domain's flow. This is what makes the
 /// attacker-supplied `d` trustworthy: the signed event can only ever travel to
 /// a host that belongs to the domain the user approved.
-fn domain_bound(callback: &str, domain: &str) -> bool {
+///
+/// Shared with [`crate::nostr::loginuri`], which applies the exact same binding
+/// so a login callback can only reach a host belonging to the approved domain.
+pub(crate) fn domain_bound(callback: &str, domain: &str) -> bool {
 	// The localhost dev callback is validated already and exempt from binding.
 	if strip_prefix_ignore_case(callback, "http://").is_some() {
 		return true;
diff --git a/src/nostr/loginuri.rs b/src/nostr/loginuri.rs
index 556692ec..b56ec550 100644
--- a/src/nostr/loginuri.rs
+++ b/src/nostr/loginuri.rs
@@ -116,6 +116,14 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
 	let challenge = validate_challenge(challenge?)?;
 	let domain = validate_domain(domain?)?;
 	let callback = validate_callback(callback?)?;
+	// Domain binding: the callback host must belong to the displayed domain, so a
+	// site cannot show `d=magick.market` while pointing `cb` at its own host and
+	// harvest a login event for a domain it does not control. Same rule as
+	// authorize (case-insensitive, ports/userinfo stripped, label-boundary
+	// subdomains allowed, http://localhost dev callback exempt).
+	if !super::authuri::domain_bound(&callback, &domain) {
+		return None;
+	}
 	Some(LoginUri {
 		challenge,
 		domain,
@@ -356,6 +364,29 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn callback_domain_binding_enforced() {
+		// d=magick.market with a callback on an unrelated host: rejected, so a
+		// login event can never be harvested by a domain the site does not control.
+		let bad = format!("goblin:login?c={C}&d=magick.market&cb=https://evil.com/x");
+		assert_eq!(parse(&bad), None, "cross-domain callback must be rejected");
+		// Host equals the domain: accepted.
+		let ok = format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb");
+		assert_eq!(parse(&ok).unwrap().callback, "https://magick.market/cb");
+		// Subdomain on a label boundary: accepted.
+		let ok = format!("goblin:login?c={C}&d=magick.market&cb=https://auth.magick.market/cb");
+		assert!(
+			parse(&ok).is_some(),
+			"label-boundary subdomain must be accepted"
+		);
+		// Suffix without a label boundary: rejected.
+		let bad = format!("goblin:login?c={C}&d=magick.market&cb=https://notmagick.market/cb");
+		assert_eq!(parse(&bad), None, "non-boundary suffix must be rejected");
+		// The http://localhost dev callback is exempt regardless of the domain.
+		let ok = format!("goblin:login?c={C}&d=magick.market&cb=http://localhost:8080/cb");
+		assert_eq!(parse(&ok).unwrap().callback, "http://localhost:8080/cb");
+	}
+
 	#[test]
 	fn percent_encoded_callback_decoded() {
 		let cb = "https%3A%2F%2Fmagick.market%2Fapi%2Flogin%3Fs%3D1";