From bfecd106507b067e60ad16177a668e762ab42acd Mon Sep 17 00:00:00 2001
From: Marcin Radomski <dextero@google.com>
Date: Tue, 18 Mar 2025 13:31:21 +0000
Subject: [PATCH 1/2] Avoid panic on buffers with embedded nul bytes

Some crates use log crate with a message padded with a number of
nullbytes [1]. This currently causes panics.

Using `CStr::from_bytes_until_nul` accepts multiple null-bytes, and
instead stops at the first nullbyte in a buffer.

This may truncate some logs with text interspersed with nullbytes.
However, I'd say logging _something_ there is a less-bad option than
crashing just because we got a nullbyte in the &str.

[1] https://github.com/cloudflare/quiche/blob/d0efd2c5278b9dbe8d6544c3015f8c772f3513b4/quiche/src/tls/mod.rs#L1040
---
 src/platform_log_writer.rs | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/platform_log_writer.rs b/src/platform_log_writer.rs
index 85826ca..6ec706d 100644
--- a/src/platform_log_writer.rs
+++ b/src/platform_log_writer.rs
@@ -123,7 +123,7 @@ impl PlatformLogWriter<'_> {
         );
 
         let initialized = unsafe { slice_assume_init_ref(&self.buffer[..len + 1]) };
-        let msg = CStr::from_bytes_with_nul(initialized)
+        let msg = CStr::from_bytes_until_nul(initialized)
             .expect("Unreachable: nul terminator was placed at `len`");
         android_log(self.buf_id, self.priority, self.tag, msg);
 
@@ -265,6 +265,23 @@ pub mod tests {
         );
     }
 
+    #[test]
+    fn output_specified_len_accepts_extra_trailing_nuls() {
+        let mut writer = get_tag_writer();
+        let log_string = "abcde\0\0\0";
+        let first_nul = log_string.find('\0').unwrap();
+        writer
+            .write_str(log_string)
+            .expect("Unable to write to PlatformLogWriter");
+
+        unsafe { writer.output_specified_len(8) };
+
+        assert_eq!(
+            unsafe { slice_assume_init_ref(&writer.buffer[..first_nul]) },
+            &log_string.as_bytes()[..first_nul]
+        );
+    }
+
     #[test]
     fn copy_bytes_to_start() {
         let mut writer = get_tag_writer();

From 58517694ff49821c11281f3957573a86b85f1b1c Mon Sep 17 00:00:00 2001
From: Marcin Radomski <dextero@google.com>
Date: Wed, 19 Mar 2025 16:25:56 +0000
Subject: [PATCH 2/2] Print nullbytes in log messages as spaces

Instead of truncating the message in output_specified_len. This way
output_specified_len still outputs specified len of bytes, while not
crashing if someone feels daring enough to log nulls.
---
 src/platform_log_writer.rs | 43 ++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/src/platform_log_writer.rs b/src/platform_log_writer.rs
index 6ec706d..2a7b53a 100644
--- a/src/platform_log_writer.rs
+++ b/src/platform_log_writer.rs
@@ -113,7 +113,7 @@ impl PlatformLogWriter<'_> {
     /// Output buffer up until the \0 which will be placed at `len` position.
     ///
     /// # Safety
-    /// The first `len` bytes of `self.buffer` must be initialized.
+    /// The first `len` bytes of `self.buffer` must be initialized and not contain nullbytes.
     unsafe fn output_specified_len(&mut self, len: usize) {
         let mut last_byte = MaybeUninit::new(b'\0');
 
@@ -123,7 +123,7 @@ impl PlatformLogWriter<'_> {
         );
 
         let initialized = unsafe { slice_assume_init_ref(&self.buffer[..len + 1]) };
-        let msg = CStr::from_bytes_until_nul(initialized)
+        let msg = CStr::from_bytes_with_nul(initialized)
             .expect("Unreachable: nul terminator was placed at `len`");
         android_log(self.buf_id, self.priority, self.tag, msg);
 
@@ -152,7 +152,13 @@ impl fmt::Write for PlatformLogWriter<'_> {
                 .zip(incoming_bytes)
                 .enumerate()
                 .fold(None, |acc, (i, (output, input))| {
-                    output.write(*input);
+                    if *input == b'\0' {
+                        // Replace nullbytes with whitespace, so we can put the message in a CStr
+                        // later to pass it through a const char*.
+                        output.write(b' ');
+                    } else {
+                        output.write(*input);
+                    }
                     if *input == b'\n' { Some(i) } else { acc }
                 });
 
@@ -265,23 +271,6 @@ pub mod tests {
         );
     }
 
-    #[test]
-    fn output_specified_len_accepts_extra_trailing_nuls() {
-        let mut writer = get_tag_writer();
-        let log_string = "abcde\0\0\0";
-        let first_nul = log_string.find('\0').unwrap();
-        writer
-            .write_str(log_string)
-            .expect("Unable to write to PlatformLogWriter");
-
-        unsafe { writer.output_specified_len(8) };
-
-        assert_eq!(
-            unsafe { slice_assume_init_ref(&writer.buffer[..first_nul]) },
-            &log_string.as_bytes()[..first_nul]
-        );
-    }
-
     #[test]
     fn copy_bytes_to_start() {
         let mut writer = get_tag_writer();
@@ -314,6 +303,20 @@ pub mod tests {
         );
     }
 
+    #[test]
+    fn writer_substitutes_nullbytes_with_spaces() {
+        let test_string = "Test_string_with\0\0\0\0nullbytes\0";
+        let mut writer = get_tag_writer();
+        writer
+            .write_str(test_string)
+            .expect("Unable to write to PlatformLogWriter");
+
+        assert_eq!(
+            unsafe { slice_assume_init_ref(&writer.buffer[..test_string.len()]) },
+            test_string.replace("\0", " ").as_bytes()
+        );
+    }
+
     fn get_tag_writer() -> PlatformLogWriter<'static> {
         PlatformLogWriter::new(
             None,