Add SSRF guard, signer queue cap, secret echo, request timeout

frontend/electron/main.ts

@@ -1,8 +1,10 @@
 import { app, BrowserWindow, clipboard, dialog, ipcMain, protocol, shell } from 'electron';
+import { lookup } from 'node:dns/promises';
 import { spawn, type ChildProcess } from 'node:child_process';
 import { randomBytes } from 'node:crypto';
 import { readFileSync } from 'node:fs';
 import { createInterface } from 'node:readline';
+import * as net from 'node:net';
 import * as path from 'node:path';
 
 const MIME: Record<string, string> = {
@@ -135,6 +137,13 @@ function startBackend(): void {
   });
 }
 
+/**
+ * Upper bound for one backend round-trip. Generous on purpose: a publish can
+ * wait for each relay in turn (10s connect + 15s send each). A hung backend
+ * still gets reaped instead of leaking promises forever.
+ */
+const BACKEND_TIMEOUT_MS = 120_000;
+
 async function backendRequest(method: string, params: Record<string, unknown>): Promise<unknown> {
   startBackend();
   const stdin = backend?.stdin;
@@ -143,11 +152,19 @@ async function backendRequest(method: string, params: Record<string, unknown>):
   }
   const id = nextId++;
   const payload = { id, method, ...params };
+  let timer!: ReturnType<typeof setTimeout>;
   const response = new Promise<unknown>((resolve, reject) => {
-    pending.set(id, { resolve, reject });
+    pending.set(id, {
+      resolve,
+      reject,
+    });
+    timer = setTimeout(() => {
+      pending.delete(id);
+      reject(new Error('The background service did not respond in time.'));
+    }, BACKEND_TIMEOUT_MS);
   });
   stdin.write(`${JSON.stringify(payload)}\n`);
-  return response;
+  return response.finally(() => clearTimeout(timer));
 }
 
 /**
@@ -219,6 +236,66 @@ const LINK_PREVIEW_TIMEOUT_MS = 10_000;
 /** Cap on how much HTML we parse for meta tags. */
 const LINK_PREVIEW_MAX_BYTES = 1_000_000;
 
+/** Whether an IPv4 address is loopback, private, or otherwise non-routable. */
+function ipv4IsPrivate(ip: string): boolean {
+  const parts = ip.split('.').map(Number);
+  if (parts.length !== 4 || parts.some((n) => Number.isNaN(n) || n < 0 || n > 255)) {
+    return true; // Malformed: treat as unsafe.
+  }
+  const [a, b] = parts;
+  return (
+    a === 0 ||
+    a === 10 ||
+    a === 127 ||
+    (a === 100 && b >= 64 && b <= 127) ||
+    (a === 169 && b === 254) ||
+    (a === 172 && b >= 16 && b <= 31) ||
+    (a === 192 && b === 168)
+  );
+}
+
+/** Whether an IPv6 address is loopback, link-local, unique-local, or v4-mapped private. */
+function ipv6IsPrivate(ip: string): boolean {
+  const addr = ip.toLowerCase();
+  if (addr === '::' || addr === '::1') {
+    return true;
+  }
+  const mapped = addr.startsWith('::ffff:') ? addr.slice(7) : null;
+  if (mapped) {
+    return net.isIPv4(mapped) ? ipv4IsPrivate(mapped) : true;
+  }
+  // fc00::/7 (unique local) and fe80::/10 (link local).
+  return /^f[cd]/.test(addr) || /^fe[89ab]/.test(addr);
+}
+
+/** Whether `ip` points at the local machine or a private network. */
+function isPrivateAddress(ip: string): boolean {
+  return net.isIPv4(ip) ? ipv4IsPrivate(ip) : ipv6IsPrivate(ip);
+}
+
+/**
+ * Resolve `url`'s host and refuse loopback/private targets, so a crafted note
+ * link cannot make the app probe the user's localhost or LAN ("SSRF"). Hosts
+ * are checked at their resolved addresses, not just by name.
+ */
+async function resolvesToPrivateAddress(url: URL): Promise<boolean> {
+  const host = url.hostname.replace(/^\[|\]$/g, '').toLowerCase();
+  if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local')) {
+    return true;
+  }
+  let addresses: string[];
+  if (net.isIP(host)) {
+    addresses = [host];
+  } else {
+    try {
+      addresses = (await lookup(host, { all: true, verbatim: true })).map((a) => a.address);
+    } catch {
+      return true; // Unresolvable: nothing useful to preview anyway.
+    }
+  }
+  return addresses.some(isPrivateAddress);
+}
+
 /** A best-effort MIME type derived from the file name. */
 function mimeForPath(filePath: string): string {
   switch (path.extname(filePath).toLowerCase()) {
@@ -344,6 +421,9 @@ async function fetchLinkPreview(rawUrl: string): Promise<LinkPreview | null> {
   if (url.protocol !== 'https:' && url.protocol !== 'http:') {
     return null;
   }
+  if (await resolvesToPrivateAddress(url)) {
+    return null;
+  }
 
   const controller = new AbortController();
   const timer = setTimeout(() => controller.abort(), LINK_PREVIEW_TIMEOUT_MS);

src/signer.rs

@@ -36,6 +36,12 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
 /// How long a request may wait for the user to approve it before it expires.
 const APPROVAL_TIMEOUT: Duration = Duration::from_secs(300);
 
+/// Maximum number of requests kept waiting for approval at once. A malicious
+/// relay flooding `sign_event` requests cannot grow the queue unboundedly or
+/// bury one genuine prompt under thousands of fakes; further requests are
+/// dropped until the user clears the queue (or entries time out).
+const MAX_PENDING_APPROVALS: usize = 20;
+
 /// Lifecycle of the remote signer, for display in the GUI.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
 #[serde(rename_all = "snake_case")]
@@ -188,6 +194,11 @@ impl Signer {
         let (sender, receiver) = oneshot::channel();
         {
             let mut inner = self.inner.lock().expect("signer mutex poisoned");
+            if inner.pending.len() >= MAX_PENDING_APPROVALS {
+                // Queue is full: drop the request instead of parking it. The
+                // caller answers with the standard "no decision" error.
+                return None;
+            }
             inner.pending.insert(
                 id.clone(),
                 PendingApprovalInner {
@@ -404,7 +415,21 @@ fn handle_request(
     signer.set_connected();
 
     match request.method.as_str() {
-        "connect" => Some(response_ok(&request.id, "ack".to_string())),
+        "connect" => {
+            // NIP-46: when the nostrconnect:// link carried a secret, the
+            // client must echo it back in its connect request, proving the
+            // link was delivered unmodified. A mismatched echo is refused.
+            if let Some(expected) = &uri.secret {
+                if !request.params.iter().any(|param| param == expected) {
+                    return Some(response_err(
+                        &request.id,
+                        "The connect acknowledgement did not include the expected secret."
+                            .to_string(),
+                    ));
+                }
+            }
+            Some(response_ok(&request.id, "ack".to_string()))
+        }
         "get_public_key" => Some(response_ok(&request.id, keys.public_key().to_hex())),
         "ping" => Some(response_ok(&request.id, "pong".to_string())),
         "sign_event" => approved_response(keys, request),
@@ -829,6 +854,62 @@ mod tests {
         assert!(!requires_approval("logout"));
     }
 
+    #[test]
+    fn connect_without_secret_is_acked() {
+        let signer = Signer::new();
+        let keys = Keys::generate();
+        let uri = ConnectUri {
+            peer: Keys::generate().public_key(),
+            relays: vec![RelayUrl::parse("wss://relay.example.com").unwrap()],
+            secret: None,
+        };
+        let request = RawRequest {
+            id: "1".into(),
+            method: "connect".into(),
+            params: vec![keys.public_key().to_hex()],
+        };
+        let response = handle_request(&signer, &keys, &uri, &request).unwrap();
+        assert!(response.contains("ack"));
+    }
+
+    #[test]
+    fn connect_must_echo_the_link_secret() {
+        let signer = Signer::new();
+        let keys = Keys::generate();
+        let uri = ConnectUri {
+            peer: Keys::generate().public_key(),
+            relays: vec![RelayUrl::parse("wss://relay.example.com").unwrap()],
+            secret: Some("shared-quiet-secret".to_string()),
+        };
+
+        // Missing echo: refused.
+        let request = RawRequest {
+            id: "1".into(),
+            method: "connect".into(),
+            params: vec![],
+        };
+        let response = handle_request(&signer, &keys, &uri, &request).unwrap();
+        assert!(response.contains("expected secret"));
+
+        // Wrong echo: refused.
+        let request = RawRequest {
+            id: "2".into(),
+            method: "connect".into(),
+            params: vec!["something-else".to_string()],
+        };
+        let response = handle_request(&signer, &keys, &uri, &request).unwrap();
+        assert!(response.contains("expected secret"));
+
+        // Correct echo: acknowledged.
+        let request = RawRequest {
+            id: "3".into(),
+            method: "connect".into(),
+            params: vec!["shared-quiet-secret".to_string()],
+        };
+        let response = handle_request(&signer, &keys, &uri, &request).unwrap();
+        assert!(response.contains("ack"));
+    }
+
     #[tokio::test]
     async fn sign_event_waits_for_approval_then_signs() {
         let signer = Signer::new();
@@ -892,6 +973,46 @@ mod tests {
         assert!(signer.approve("no-such-id", true).is_err());
     }
 
+    #[tokio::test]
+    async fn pending_approvals_are_capped() {
+        let signer = Signer::new();
+        let keys = Keys::generate();
+        let mk_request = |id: usize| RawRequest {
+            id: id.to_string(),
+            method: "sign_event".into(),
+            params: vec![r#"{"kind":1,"created_at":1714078911,"tags":[],"content":"x"}"#.into()],
+        };
+
+        // Flood with more gated requests than the cap allows.
+        let mut tasks = Vec::new();
+        for i in 0..MAX_PENDING_APPROVALS + 5 {
+            let s1 = signer.clone();
+            let keys_for_task = keys.clone();
+            let request = mk_request(i);
+            tasks.push(tokio::spawn(async move {
+                gated_response(&s1, &keys_for_task, &request).await
+            }));
+        }
+        tokio::time::sleep(Duration::from_millis(50)).await;
+
+        assert_eq!(
+            signer.pending_count(),
+            MAX_PENDING_APPROVALS,
+            "queue must never exceed the cap"
+        );
+
+        // Approve one; the queue drains by exactly one (overflow requests were
+        // already dropped, not parked).
+        let pending_id = signer.status().pending[0].id.clone();
+        signer.approve(&pending_id, false).unwrap();
+        tokio::time::sleep(Duration::from_millis(20)).await;
+        assert_eq!(signer.pending_count(), MAX_PENDING_APPROVALS - 1);
+
+        for task in tasks {
+            task.abort();
+        }
+    }
+
     #[test]
     fn sign_event_summary_previews_the_event() {
         let request = RawRequest {