Fix missing NUL-termination in Demangle() on parse failure and make ParseAbiTags() restore pre-parse state correctly

PiperOrigin-RevId: 947849322
Change-Id: I899ab7a5fa16f478fdfa7765a36418224208f1d9

Author: Abseil Team

absl/debugging/internal/demangle.cc

@@ -17,6 +17,7 @@
 
 #include "absl/debugging/internal/demangle.h"
 
+#include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <cstdio>
@@ -456,22 +457,35 @@ static bool ZeroOrMore(ParseFunc parse_func, State *state) {
 }
 
 // Append "str" at "out_cur_idx".  If there is an overflow, out_cur_idx is
-// set to out_end_idx+1.  The output string is ensured to
-// always terminate with '\0' as long as there is no overflow.
+// set to out_end_idx+1.  The output buffer is always terminated with '\0' if it
+// has nonzero length.
 static void Append(State *state, const char *const str, const size_t length) {
-  for (size_t i = 0; i < length; ++i) {
-    if (state->parse_state.out_cur_idx + 1 <
-        state->out_end_idx) {  // +1 for '\0'
-      state->out[state->parse_state.out_cur_idx++] = str[i];
-    } else {
-      // signal overflow
-      state->parse_state.out_cur_idx = state->out_end_idx + 1;
-      break;
-    }
+  if (length == 0) {
+    return;
+  }
+
+  // Figure out how much space is remaining in the output buffer to copy into.
+  const int cap = state->out_end_idx - state->parse_state.out_cur_idx;
+
+  // If overflow was already signaled (negative value, set further below) or
+  // there is zero space to write into, we cannot do anything.
+  if (cap <= 0) {
+    return;
   }
-  if (state->parse_state.out_cur_idx < state->out_end_idx) {
-    state->out[state->parse_state.out_cur_idx] =
-        '\0';  // Terminate it with '\0'
+
+  // Copy the number of characters requested, capped by the amount of space
+  // remaining.
+  std::char_traits<char>::copy(state->out + state->parse_state.out_cur_idx, str,
+                               (std::min)(length, static_cast<size_t>(cap)));
+
+  // Did we copy everything we needed to, with enough room to NUL-terminate?
+  if (length < static_cast<size_t>(cap)) {
+    state->parse_state.out_cur_idx += static_cast<int>(length);
+    state->out[state->parse_state.out_cur_idx] = '\0';
+  } else {
+    // No, we ran out of space. Signal overflow, and NUL-terminate for safety.
+    state->parse_state.out_cur_idx = state->out_end_idx + 1;
+    state->out[state->out_end_idx - 1] = '\0';
   }
 }
 
@@ -893,8 +907,11 @@ static bool ParseAbiTags(State *state) {
   ComplexityGuard guard(state);
   if (guard.IsTooComplex()) return false;
 
-  while (ParseOneCharToken(state, 'B')) {
-    ParseState copy = state->parse_state;
+  for (;;) {
+    const ParseState copy = state->parse_state;
+    if (!ParseOneCharToken(state, 'B')) {
+      break;
+    }
     MaybeAppend(state, "[abi:");
 
     if (!ParseSourceName(state)) {

absl/debugging/internal/demangle_test.cc

@@ -14,6 +14,7 @@
 
 #include "absl/debugging/internal/demangle.h"
 
+#include <array>
 #include <cstdlib>
 #include <memory>
 #include <string>
@@ -30,6 +31,7 @@ ABSL_NAMESPACE_BEGIN
 namespace debugging_internal {
 namespace {
 
+using ::testing::Contains;
 using ::testing::ContainsRegex;
 
 TEST(Demangle, FunctionTemplate) {
@@ -1908,6 +1910,13 @@ TEST(Demangle, DelegatesToDemangleRustSymbolEncoding) {
   EXPECT_STREQ("my_crate::my_func", tmp);
 }
 
+TEST(Demangle, DemanglingNulTerminatesOnParsingFailure) {
+  std::array buf = {'\xAA', '\xAA', '\xAA', '\xAA'};
+  EXPECT_FALSE(Demangle("_ZN1xBE", std::data(buf), std::size(buf)));
+  // Ensure string is properly NUL-terminated despite parsing failure.
+  EXPECT_THAT(buf, Contains('\0'));
+}
+
 // Tests that verify that Demangle footprint is within some limit.
 // They are not to be run under sanitizers as the sanitizers increase
 // stack consumption by about 4x.