fix(command): harden length-delimited + SCM IPC against panic-OOB

command/src/channel.rs

@@ -42,6 +42,13 @@ pub enum ChannelError {
         capacity: usize,
         max: usize,
     },
+    #[error(
+        "declared message length ({message_len} bytes) is shorter than the {delimiter_size}-byte length prefix"
+    )]
+    MessageLengthUnderDelimiter {
+        message_len: usize,
+        delimiter_size: usize,
+    },
     #[error("channel could not write on the back buffer")]
     Write(std::io::Error),
     #[error("channel buffer is full ({capacity} bytes, max {max} bytes), cannot grow more")]
@@ -472,6 +479,25 @@ impl<Tx: Debug + ProstMessage + Default, Rx: Debug + ProstMessage + Default> Cha
                 });
             }
 
+            // A length-delimited frame is `[delimiter][payload]`. The declared
+            // `message_len` is the total frame size and MUST therefore be at
+            // least `delimiter_size()`. A peer-controlled value below that
+            // ceiling makes `&buffer[delimiter_size()..message_len]` slice
+            // backwards and panic; reject it the same way as oversized frames.
+            //
+            // Drop the bogus delimiter bytes before returning so the channel
+            // can re-sync on the peer's next frame. Without this, every
+            // subsequent `read_message()` re-reads the same bad header from
+            // the front buffer and the worker burns CPU on the same error
+            // until the peer disconnects.
+            if message_len < delimiter_size() {
+                self.front_buf.consume(delimiter_size());
+                return Err(ChannelError::MessageLengthUnderDelimiter {
+                    message_len,
+                    delimiter_size: delimiter_size(),
+                });
+            }
+
             if buffer.len() >= message_len {
                 let message = Rx::decode(&buffer[delimiter_size()..message_len])
                     .map_err(ChannelError::InvalidProtobufMessage)?;
@@ -962,4 +988,43 @@ mod tests {
             "expected doubling growth pattern, got {grown}"
         );
     }
+
+    /// Regression: a peer that writes a length-delimited frame whose
+    /// declared length is *less than* the delimiter itself must be
+    /// rejected with `MessageLengthUnderDelimiter`, never panic the
+    /// reader with `slice index starts at N but ends at M`.
+    ///
+    /// Without the bounds check, `&buffer[delimiter_size()..message_len]`
+    /// at `try_read_delimited_message` panics for any peer-controlled
+    /// `message_len < delimiter_size()` (= 8 on 64-bit) — a one-packet
+    /// denial-of-service against the master command socket.
+    #[test]
+    fn rejects_declared_length_below_delimiter() {
+        let (mut reader, mut writer): (
+            Channel<ProtobufMessage, ProtobufMessage>,
+            Channel<ProtobufMessage, ProtobufMessage>,
+        ) = Channel::generate(1000, 10000).expect("could not generate channels");
+        writer.blocking().expect("writer to block");
+        reader.blocking().expect("reader to block");
+
+        // Craft a delimiter that lies: message_len = 5 (< delimiter_size() = 8).
+        // Send it as raw bytes, bypassing write_delimited_message.
+        let bogus: usize = 5;
+        let bytes = bogus.to_le_bytes();
+        std::io::Write::write_all(&mut writer.sock, &bytes).expect("raw write of bogus delimiter");
+
+        match reader.read_message() {
+            Err(ChannelError::MessageLengthUnderDelimiter {
+                message_len,
+                delimiter_size,
+            }) => {
+                assert_eq!(message_len, 5);
+                assert_eq!(delimiter_size, std::mem::size_of::<usize>());
+            }
+            other => panic!(
+                "expected MessageLengthUnderDelimiter, got {other:?}\n\
+                 NOTE: a panic here means the slice-OOB hardening was reverted",
+            ),
+        }
+    }
 }

command/src/scm_socket.rs

@@ -47,6 +47,18 @@ pub enum ScmSocketError {
     },
     #[error("error decoding the protobuf format of the listeners: {0}")]
     DecodeError(DecodeError),
+    #[error(
+        "listeners count manifest is inconsistent with the SCM payload: \
+         http={http}, tls={tls}, tcp={tcp} (sum={total}), fds_received={fds_received}, max_fds={max_fds}"
+    )]
+    ListenersCountInconsistent {
+        http: usize,
+        tls: usize,
+        tcp: usize,
+        total: usize,
+        fds_received: usize,
+        max_fds: usize,
+    },
 }
 
 /// A unix socket specialized for file descriptor passing
@@ -135,12 +147,43 @@ impl ScmSocket {
         let listeners_count = ListenersCount::decode_length_delimited(&buf[..size])
             .map_err(ScmSocketError::DecodeError)?;
 
+        // Validate the manifest before indexing into the fixed-size FD array.
+        // The peer-controlled `listeners_count.{http,tls,tcp}` lists are
+        // matched 1:1 with `received_fds` slots; without these bounds checks
+        // a peer that declared more entries than MAX_FDS_OUT or more entries
+        // than FDs actually arrived would panic the worker on
+        // `received_fds[index..index + len]`.
+        let http_len = listeners_count.http.len();
+        let tls_len = listeners_count.tls.len();
+        let tcp_len = listeners_count.tcp.len();
+        let total = http_len
+            .checked_add(tls_len)
+            .and_then(|s| s.checked_add(tcp_len))
+            .ok_or(ScmSocketError::ListenersCountInconsistent {
+                http: http_len,
+                tls: tls_len,
+                tcp: tcp_len,
+                total: usize::MAX,
+                fds_received: file_descriptor_length,
+                max_fds: MAX_FDS_OUT,
+            })?;
+        if total > MAX_FDS_OUT || total > file_descriptor_length {
+            return Err(ScmSocketError::ListenersCountInconsistent {
+                http: http_len,
+                tls: tls_len,
+                tcp: tcp_len,
+                total,
+                fds_received: file_descriptor_length,
+                max_fds: MAX_FDS_OUT,
+            });
+        }
+
         let mut http_addresses = parse_addresses(&listeners_count.http)?;
         let mut tls_addresses = parse_addresses(&listeners_count.tls)?;
         let mut tcp_addresses = parse_addresses(&listeners_count.tcp)?;
 
         let mut index = 0;
-        let len = listeners_count.http.len();
+        let len = http_len;
         let mut http = Vec::new();
         http.extend(
             http_addresses
@@ -149,7 +192,7 @@ impl ScmSocket {
         );
 
         index += len;
-        let len = listeners_count.tls.len();
+        let len = tls_len;
         let mut tls = Vec::new();
         tls.extend(
             tls_addresses
@@ -158,11 +201,12 @@ impl ScmSocket {
         );
 
         index += len;
+        let len = tcp_len;
         let mut tcp = Vec::new();
         tcp.extend(
             tcp_addresses
                 .drain(..)
-                .zip(received_fds[index..file_descriptor_length].iter().cloned()),
+                .zip(received_fds[index..index + len].iter().cloned()),
         );
 
         Ok(Listeners { http, tls, tcp })
@@ -427,4 +471,57 @@ mod tests {
 
         assert_eq!(listeners.http[0].0, received_listeners.http[0].0);
     }
+
+    /// Regression: a malformed `ListenersCount` whose entry counts do not
+    /// match the number of file descriptors received over SCM must be
+    /// rejected with `ListenersCountInconsistent`, never panic the worker
+    /// on `received_fds[index..index + len]`.
+    ///
+    /// Without the bounds check, a peer that declares more addresses than
+    /// `MAX_FDS_OUT` (or more than the FDs that actually arrived) crashes
+    /// the receiving worker on out-of-bounds array indexing.
+    #[test]
+    fn rejects_listeners_count_with_more_entries_than_fds() {
+        let (stream_1, stream_2) =
+            MioUnixStream::pair().expect("Could not create a pair of mio unix streams");
+        let sender = ScmSocket::new(stream_1.into_raw_fd()).expect("Could not create scm socket");
+        let receiver = ScmSocket::new(stream_2.into_raw_fd()).expect("Could not create scm socket");
+
+        // Declare three HTTP entries but ship zero file descriptors.
+        let bogus = ListenersCount {
+            http: vec![
+                "127.0.0.1:80".to_string(),
+                "127.0.0.2:80".to_string(),
+                "127.0.0.3:80".to_string(),
+            ],
+            tls: vec![],
+            tcp: vec![],
+        };
+        let payload = bogus.encode_length_delimited_to_vec();
+        sender
+            .send_msg_and_fds(&payload, &[])
+            .expect("manual send_msg_and_fds with zero fds must succeed at the syscall layer");
+
+        match receiver.receive_listeners() {
+            Err(ScmSocketError::ListenersCountInconsistent {
+                http,
+                tls,
+                tcp,
+                total,
+                fds_received,
+                max_fds,
+            }) => {
+                assert_eq!(http, 3);
+                assert_eq!(tls, 0);
+                assert_eq!(tcp, 0);
+                assert_eq!(total, 3);
+                assert_eq!(fds_received, 0);
+                assert_eq!(max_fds, MAX_FDS_OUT);
+            }
+            other => panic!(
+                "expected ListenersCountInconsistent, got {other:?}\n\
+                 NOTE: a panic / OOM here means the SCM bounds check was reverted",
+            ),
+        }
+    }
 }

e2e/Cargo.toml

@@ -36,6 +36,16 @@ tempfile = { workspace = true }
 # request consume the failure injection that FIX-18 just armed,
 # producing the wrong response code for both tests on CI.
 serial_test = { workspace = true }
+# Encode forged length-delimited protobuf payloads in
+# `tests/command_channel_security_tests.rs` to reproduce the
+# `ScmSocket::receive_listeners` slice-OOB pre-fix and prove the
+# `ListenersCountInconsistent` rejection path post-fix.
+prost = { workspace = true }
+# `sendmsg` syscall wrapper used by the same test to ship a
+# malformed `ListenersCount` over the SCM socket without going
+# through `ScmSocket::send_listeners` (which keeps addresses and
+# FDs in sync by construction).
+nix = { workspace = true, features = ["socket", "uio"] }
 
 [features]
 # Forward sozu-lib's `splice` feature so the dependency-tree compiles