BytesToHex should accept null buffers if length is 0. (#19827)

Fixes #19727
project-chip · Feb 9, 2024 · 915caec · 915caec
1 parent 532791f
commit 915caec
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 12 deletions.
diff --git a/src/lib/support/BytesToHex.cpp b/src/lib/support/BytesToHex.cpp
@@ -98,10 +98,16 @@ size_t HexToBytes(const char * src_hex, const size_t src_size, uint8_t * dest_by
 
 CHIP_ERROR BytesToHex(const uint8_t * src_bytes, size_t src_size, char * dest_hex, size_t dest_size_max, BitFlags<HexFlags> flags)
 {
-    if ((src_bytes == nullptr) || (dest_hex == nullptr))
+    if ((src_bytes == nullptr) && (src_size != 0))
     {
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
+
+    if ((dest_hex == nullptr) && (dest_size_max != 0))
+    {
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
     if (src_size > ((SIZE_MAX - 1) / 2u))
     {
         // Output would overflow a size_t, let's bail out to avoid computation wraparounds below.

diff --git a/src/lib/support/BytesToHex.h b/src/lib/support/BytesToHex.h
@@ -59,15 +59,18 @@ enum class HexFlags : int
  * On success, number of bytes written to destination is always:
  *   output_size = (src_size * 2) + ((flags & HexFlags::kNullTerminate) ? 1 : 0);
  *
- * @param src_bytes Pointer to non-null buffer to convert
+ * @param src_bytes Pointer to buffer to convert.  Only allowed to be null if
+ *                  src_size is 0.
  * @param src_size Number of bytes to convert from src_bytes
  * @param [out] dest_hex Destination buffer to receive hex encoding
  * @param dest_size_max Maximum buffer size for the hex encoded `dest_hex` buffer
  *                      including null-terminator if needed.
  * @param flags Flags from `HexFlags` for formatting options
  *
  * @return CHIP_ERROR_BUFFER_TOO_SMALL on dest_max_size too small to fit output
- * @return CHIP_ERROR_INVALID_ARGUMENT if either src_bytes or dest_hex is nullptr
+ * @return CHIP_ERROR_INVALID_ARGUMENT if either src_bytes or dest_hex is
+ *                                     nullptr without the corresponding size
+ *                                     being 0.
  * @return CHIP_NO_ERROR on success
  */
 

diff --git a/src/lib/support/tests/TestBytesToHex.cpp b/src/lib/support/tests/TestBytesToHex.cpp
@@ -66,6 +66,21 @@ void TestBytesToHexNotNullTerminated(nlTestSuite * inSuite, void * inContext)
         // Nothing should have been touched.
         NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
     }
+
+    // Trivial: Zero size input with null buffer
+    {
+        char dest[2]     = { '!', '@' };
+        char expected[2] = { '!', '@' };
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, &dest[0], sizeof(dest), HexFlags::kNone) == CHIP_NO_ERROR);
+        // Nothing should have been touched.
+        NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
+
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, nullptr, 0, HexFlags::kNone) == CHIP_NO_ERROR);
+        // Nothing should have been touched.
+        NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
+
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, nullptr, 1, HexFlags::kNone) == CHIP_ERROR_INVALID_ARGUMENT);
+    }
 }
 
 void TestBytesToHexNullTerminated(nlTestSuite * inSuite, void * inContext)
@@ -115,21 +130,27 @@ void TestBytesToHexNullTerminated(nlTestSuite * inSuite, void * inContext)
         // Expect nul termination
         NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
     }
-}
 
-void TestBytesToHexErrors(nlTestSuite * inSuite, void * inContext)
-{
-    // NULL source
+    // Trivial: Zero size input with null buffer
     {
-        const uint8_t * src = nullptr;
-        char dest[18]       = { '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '!', '@' };
-        char expected[18]   = { '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '?', '!', '@' };
-        NL_TEST_ASSERT(inSuite, BytesToHex(&src[0], 0, &dest[0], sizeof(dest), HexFlags::kNone) == CHIP_ERROR_INVALID_ARGUMENT);
+        char dest[2]     = { '!', '@' };
+        char expected[2] = { '\0', '@' };
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, &dest[0], sizeof(dest), HexFlags::kNullTerminate) == CHIP_NO_ERROR);
+        // Nothing should have been touched.
+        NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
 
-        // Buffers should not have been touched
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, nullptr, 0, HexFlags::kNullTerminate) == CHIP_ERROR_BUFFER_TOO_SMALL);
+
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, &dest[0], 1, HexFlags::kNullTerminate) == CHIP_NO_ERROR);
+        // Nothing should have been touched.
         NL_TEST_ASSERT(inSuite, memcmp(&dest[0], &expected[0], sizeof(expected)) == 0);
+
+        NL_TEST_ASSERT(inSuite, BytesToHex(nullptr, 0, nullptr, 1, HexFlags::kNullTerminate) == CHIP_ERROR_INVALID_ARGUMENT);
     }
+}
 
+void TestBytesToHexErrors(nlTestSuite * inSuite, void * inContext)
+{
     // NULL destination
     {
         uint8_t src[] = { 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF };