[lldb] Fix out of bounds read in DataExtractor::GetCStr and add unit test that function.

Summary:
The `if (*cstr_end == '\0')` in the previous code checked if the previous loop terminated because it
found a null terminator or because it reached the end of the data. However, in the case that we hit
the end of the data before finding a null terminator, `cstr_end` points behind the last byte in our
data and `*cstr_end` reads the memory behind the array (which may be uninitialised)

This patch just rewrites that function use `std::find` and adds the relevant unit tests.

Reviewers: labath

Reviewed By: labath

Subscribers: abidh, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68773

llvm-svn: 374311

1 files changed, 18 insertions, 19 deletions

diff --git a/lldb/source/Utility/DataExtractor.cpp b/lldb/source/Utility/DataExtractor.cpp
index 79a1f75d737..f642a8fc763 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -816,26 +816,25 @@ DataExtractor::CopyByteOrderedData(offset_t src_offset, offset_t src_len,
 // non-zero and there aren't enough available bytes, nullptr will be returned
 // and "offset_ptr" will not be updated.
 const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
-  const char *cstr = reinterpret_cast<const char *>(PeekData(*offset_ptr, 1));
-  if (cstr) {
-    const char *cstr_end = cstr;
-    const char *end = reinterpret_cast<const char *>(m_end);
-    while (cstr_end < end && *cstr_end)
-      ++cstr_end;
-
-    // Now we are either at the end of the data or we point to the
-    // NULL C string terminator with cstr_end...
-    if (*cstr_end == '\0') {
-      // Advance the offset with one extra byte for the NULL terminator
-      *offset_ptr += (cstr_end - cstr + 1);
-      return cstr;
-    }
+  const char *start = reinterpret_cast<const char *>(PeekData(*offset_ptr, 1));
+  // Already at the end of the data.
+  if (!start)
+    return nullptr;
 
-    // We reached the end of the data without finding a NULL C string
-    // terminator. Fall through and return nullptr otherwise anyone that would
-    // have used the result as a C string can wander into unknown memory...
-  }
-  return nullptr;
+  const char *end = reinterpret_cast<const char *>(m_end);
+
+  // Check all bytes for a null terminator that terminates a C string.
+  const char *terminator_or_end = std::find(start, end, '\0');
+
+  // We didn't find a null terminator, so return nullptr to indicate that there
+  // is no valid C string at that offset.
+  if (terminator_or_end == end)
+    return nullptr;
+
+  // Update offset_ptr for the caller to point to the data behind the
+  // terminator (which is 1 byte long).
+  *offset_ptr += (terminator_or_end - start + 1UL);
+  return start;
 }
 
 // Extracts a NULL terminated C string from the fixed length field of length