Merge pull request #8956 from dylanmtaylor/security/ipp-reply-deadline

security(ipp): total reply deadline, resumable chunk walk

crates/ipp/src/lib.rs

@@ -67,6 +67,8 @@ pub struct Printer {
 /// Timeouts: connecting, and each request (a Print-Job carries the whole PDF).
 const CONNECT: Duration = Duration::from_secs(3);
 const REQUEST: Duration = Duration::from_secs(60);
+/// The whole reply, however it trickles in: the timeouts above only cover each read.
+const REPLY_DEADLINE: Duration = Duration::from_secs(120);
 /// Largest reply read.
 const MAX_REPLY: usize = 16 << 20;
 /// Largest number of attributes taken from a reply.
@@ -434,7 +436,13 @@ fn read_response(stream: &mut Stream) -> Result<Vec<u8>> {
     let mut body_at = None;
     let mut expected: Option<usize> = None;
     let mut chunked = false;
+    // Where the chunk walk resumes, so each read costs its own bytes, not the whole body again.
+    let mut chunk_at = 0usize;
+    let started = std::time::Instant::now();
     loop {
+        if started.elapsed() > REPLY_DEADLINE {
+            return Err(Error::Protocol("the print server took too long to reply".into()));
+        }
         let n = stream.read(&mut buf).map_err(|e| Error::Protocol(format!("reading: {e}")))?;
         if n == 0 {
             return Ok(raw);
@@ -455,7 +463,7 @@ fn read_response(stream: &mut Stream) -> Result<Vec<u8>> {
         let body = raw.get(at..).unwrap_or_default();
         let complete = match expected {
             Some(len) if !chunked => body.len() >= len,
-            _ if chunked => chunked_complete(body),
+            _ if chunked => chunked_complete(body, &mut chunk_at),
             _ => false,
         };
         if complete {
@@ -483,8 +491,11 @@ fn http_response(raw: &[u8]) -> Result<(u16, Headers, &[u8])> {
 /// `0\r\n\r\n` inside a chunk's payload (an attribute value can hold it) does not count. A
 /// malformed or impossible size counts as complete, so [`dechunk`] reports it instead of
 /// waiting for more.
-fn chunked_complete(mut b: &[u8]) -> bool {
+fn chunked_complete(body: &[u8], resume: &mut usize) -> bool {
+    let mut b = body.get(*resume..).unwrap_or_default();
     loop {
+        // Chunks already walked are not walked again: `resume` marks the start of this one.
+        *resume = body.len() - b.len();
         let Some(line_end) = b.windows(2).position(|w| w == b"\r\n") else { return false };
         let size_str = String::from_utf8_lossy(b.get(..line_end).unwrap_or(&[]));
         let Ok(size) = usize::from_str_radix(size_str.split(';').next().unwrap_or_default().trim(), 16) else { return true };

crates/ipp/src/tests.rs

@@ -429,10 +429,29 @@ fn a_last_chunk_pattern_inside_a_value_does_not_end_the_reply() {
     assert_eq!(list.len(), 1);
     assert_eq!(list[0].name, "Of\r\n0\r\n\r\nfice");
     // The boundary walk itself: complete only at a zero-size chunk, never on payload bytes.
-    assert!(!chunked_complete(b"5\r\nab\r\n0"));
-    assert!(!chunked_complete(b"7\r\n\r\n0\r\n\r\nx\r\n"));
-    assert!(chunked_complete(b"7\r\n\r\n0\r\n\r\nx\r\n0\r\n\r\n"));
-    assert!(chunked_complete(b"zz\r\n") && dechunk(b"zz\r\n").is_err());
+    assert!(!complete(b"5\r\nab\r\n0"));
+    assert!(!complete(b"7\r\n\r\n0\r\n\r\nx\r\n"));
+    assert!(complete(b"7\r\n\r\n0\r\n\r\nx\r\n0\r\n\r\n"));
+    assert!(complete(b"zz\r\n") && dechunk(b"zz\r\n").is_err());
+}
+
+/// [`chunked_complete`] on a whole body at once.
+fn complete(body: &[u8]) -> bool {
+    chunked_complete(body, &mut 0)
+}
+
+/// Fed a byte at a time, as a server dripping the reply would, the walk resumes where it stopped
+/// and agrees with the one-shot answer at every length.
+#[test]
+fn the_chunk_walk_resumes_instead_of_starting_over() {
+    let body = b"3\r\nabc\r\n10\r\n0123456789abcdef\r\n0\r\n\r\n";
+    let mut at = 0;
+    for len in 0..=body.len() {
+        let prefix = &body[..len];
+        assert_eq!(chunked_complete(prefix, &mut at), complete(prefix), "{len}");
+        assert!(at <= len);
+    }
+    assert!(at > 0, "it moved past the chunks already walked");
 }
 
 /// A chunk size that cannot be satisfied (`usize::MAX`, `usize::MAX - 1`, anything past the
@@ -440,7 +459,7 @@ fn a_last_chunk_pattern_inside_a_value_does_not_end_the_reply() {
 #[test]
 fn an_impossible_chunk_size_is_a_protocol_error_not_a_panic() {
     for size_line in ["ffffffffffffffff", "fffffffffffffffe", "1000001"] {
-        assert!(chunked_complete(format!("{size_line}\r\n").as_bytes()));
+        assert!(complete(format!("{size_line}\r\n").as_bytes()));
         assert!(dechunk(format!("{size_line}\r\nab\r\n").as_bytes()).is_err());
         let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
         let port = listener.local_addr().unwrap().port();