stabilize ids and harden release pipeline

crates/browser-signer-proxy/src/lib.rs

@@ -26,6 +26,13 @@ pub use self::error::Error;
 const DEFAULT_HTML: &str = include_str!("../index.html");
 const JS: &str = include_str!("../proxy.js");
 
+/// Cap on the request line and headers so a peer cannot force unbounded reads
+const MAX_HEADER_BYTES: usize = 16 * 1024;
+/// Cap on the request body so a huge Content-Length cannot force a huge allocation
+const MAX_BODY_BYTES: usize = 1024 * 1024;
+/// Maximum time a connection may stay open before it is dropped
+const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30);
+
 type PendingResponseMap = HashMap<Uuid, channel::Sender<Result<Value, String>>>;
 
 #[derive(Debug, Deserialize)]
@@ -303,12 +310,20 @@ impl BrowserSignerProxy {
 
                             smol::future::or(
                                 async {
-                                    handle_connection(
-                                        stream,
-                                        state,
-                                        custom_html,
-                                        expected_host,
-                                        session_token,
+                                    smol::future::or(
+                                        async {
+                                            handle_connection(
+                                                stream,
+                                                state,
+                                                custom_html,
+                                                expected_host,
+                                                session_token,
+                                            )
+                                            .await;
+                                        },
+                                        async {
+                                            smol::Timer::after(CONNECTION_TIMEOUT).await;
+                                        },
                                     )
                                     .await;
                                 },
@@ -401,10 +416,19 @@ impl BrowserSignerProxy {
 
     #[inline]
     async fn _sign_event(&self, event: UnsignedEvent) -> Result<Event, Error> {
+        let expected_id: EventId = event.compute_id();
         let event: Event = self
             .request(RequestMethod::SignEvent, serde_json::to_value(event)?)
             .await?;
         event.verify()?;
+
+        if event.id != expected_id {
+            return Err(Error::generic(format!(
+                "signed event doesn't match the requested event: expected ID {expected_id}, got {}",
+                event.id
+            )));
+        }
+
         Ok(event)
     }
 
@@ -466,6 +490,54 @@ impl AsyncNip44 for BrowserSignerProxy {
     }
 }
 
+/// Outcome of reading a line with [`read_line_capped`].
+enum HeaderLine {
+    /// A line, including the trailing newline when present
+    Line(Vec<u8>),
+    /// Clean EOF with no data
+    Eof,
+    /// The line exceeded the allowed length
+    TooLong,
+}
+
+/// Read one `\n`-terminated line with a hard cap so a peer cannot force unbounded buffering.
+async fn read_line_capped(
+    reader: &mut BufReader<TcpStream>,
+    limit: usize,
+) -> Result<HeaderLine, std::io::Error> {
+    let mut line: Vec<u8> = Vec::new();
+    loop {
+        let (data, line_complete) = {
+            let chunk = reader.fill_buf().await?;
+            if chunk.is_empty() {
+                break;
+            }
+            match chunk.iter().position(|byte| *byte == b'\n') {
+                Some(index) => (chunk[..=index].to_vec(), true),
+                None => (chunk.to_vec(), false),
+            }
+        };
+
+        if line.len() + data.len() > limit {
+            return Ok(HeaderLine::TooLong);
+        }
+
+        let taken = data.len();
+        line.extend_from_slice(&data);
+        reader.consume(taken);
+
+        if line_complete {
+            return Ok(HeaderLine::Line(line));
+        }
+    }
+
+    Ok(if line.is_empty() {
+        HeaderLine::Eof
+    } else {
+        HeaderLine::Line(line)
+    })
+}
+
 /// Handle a single HTTP connection.
 async fn handle_connection(
     stream: TcpStream,
@@ -475,13 +547,26 @@ async fn handle_connection(
     session_token: String,
 ) {
     let mut reader = BufReader::new(stream);
+    let mut header_bytes: usize = 0;
 
     // Read the request line
-    let mut request_line = String::new();
-    if reader.read_line(&mut request_line).await.is_err() {
-        return;
-    }
-    let request_line = request_line.trim_end().to_string();
+    let request_line = match read_line_capped(&mut reader, MAX_HEADER_BYTES).await {
+        Ok(HeaderLine::Line(line)) => line,
+        Ok(HeaderLine::Eof) => Vec::new(),
+        Ok(HeaderLine::TooLong) => {
+            send_response(&mut reader, 400, "Bad Request", "", "").await;
+            return;
+        }
+        Err(_) => return,
+    };
+    header_bytes += request_line.len();
+    let request_line = match String::from_utf8(request_line) {
+        Ok(line) => line.trim_end().to_string(),
+        Err(_) => {
+            send_response(&mut reader, 400, "Bad Request", "", "").await;
+            return;
+        }
+    };
 
     // Parse method, path, and HTTP version from request line
     let parts: Vec<&str> = request_line.split_whitespace().collect();
@@ -498,12 +583,29 @@ async fn handle_connection(
     let mut host: Option<String> = None;
 
     loop {
-        let mut line = String::new();
-        if reader.read_line(&mut line).await.is_err() {
+        let line = match read_line_capped(&mut reader, MAX_HEADER_BYTES).await {
+            Ok(HeaderLine::Line(line)) => line,
+            Ok(HeaderLine::Eof) => break,
+            Ok(HeaderLine::TooLong) => {
+                send_response(&mut reader, 400, "Bad Request", "", "").await;
+                return;
+            }
+            Err(_) => return,
+        };
+
+        header_bytes += line.len();
+        if header_bytes > MAX_HEADER_BYTES {
+            send_response(&mut reader, 400, "Bad Request", "", "").await;
             return;
         }
 
-        let line = line.trim_end().to_string();
+        let line = match String::from_utf8(line) {
+            Ok(line) => line.trim_end().to_string(),
+            Err(_) => {
+                send_response(&mut reader, 400, "Bad Request", "", "").await;
+                return;
+            }
+        };
         if line.is_empty() {
             break;
         }
@@ -521,6 +623,12 @@ async fn handle_connection(
         headers.push(line);
     }
 
+    // Reject oversized bodies before allocating the read buffer
+    if content_length > MAX_BODY_BYTES {
+        send_response(&mut reader, 413, "Payload Too Large", "", "").await;
+        return;
+    }
+
     // DNS rebinding defense: only the exact bound host:port is accepted
     if host.as_deref() != Some(expected_host.as_str()) {
         send_response(&mut reader, 403, "Forbidden", "", "").await;
@@ -594,7 +702,17 @@ async fn handle_connection(
                 }
             };
 
-            tracing::debug!("Received response from browser: {message:?}");
+            // Never log the response value, it can contain decrypted plaintext
+            let outcome = if message.error.is_some() {
+                "error"
+            } else {
+                "ok"
+            };
+
+            tracing::debug!(
+                "Received response from browser: id={} outcome={outcome}",
+                message.id
+            );
 
             let id: Uuid = message.id;
             let mut pending = state.pending_responses.lock().await;