From 27c30ec671cb7b5ba84c4e79feff7fd0b0ac6338 Mon Sep 17 00:00:00 2001
From: Roman Gershman <romange@gmail.com>
Date: Thu, 23 May 2019 06:03:35 +0300
Subject: [PATCH] Avoid undefined behavior when nullptr is passed to memcpy
 with size 0

---
 absl/strings/str_cat.cc      | 16 +++++++++++-----
 absl/strings/str_cat_test.cc | 14 ++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/absl/strings/str_cat.cc b/absl/strings/str_cat.cc
index 2667976d9..ffe99db81 100644
--- a/absl/strings/str_cat.cc
+++ b/absl/strings/str_cat.cc
@@ -89,7 +89,9 @@ static char* Append(char* out, const AlphaNum& x) {
   // memcpy is allowed to overwrite arbitrary memory, so doing this after the
   // call would force an extra fetch of x.size().
   char* after = out + x.size();
-  memcpy(out, x.data(), x.size());
+  if (x.size() != 0) {
+    memcpy(out, x.data(), x.size());
+  }
   return after;
 }
 
@@ -146,8 +148,10 @@ std::string CatPieces(std::initializer_list<absl::string_view> pieces) {
   char* out = begin;
   for (const absl::string_view piece : pieces) {
     const size_t this_size = piece.size();
-    memcpy(out, piece.data(), this_size);
-    out += this_size;
+    if (this_size != 0) {
+      memcpy(out, piece.data(), this_size);
+      out += this_size;
+    }
   }
   assert(out == begin + result.size());
   return result;
@@ -176,8 +180,10 @@ void AppendPieces(std::string* dest,
   char* out = begin + old_size;
   for (const absl::string_view piece : pieces) {
     const size_t this_size = piece.size();
-    memcpy(out, piece.data(), this_size);
-    out += this_size;
+    if (this_size != 0) {
+      memcpy(out, piece.data(), this_size);
+      out += this_size;
+    }
   }
   assert(out == begin + dest->size());
 }
diff --git a/absl/strings/str_cat_test.cc b/absl/strings/str_cat_test.cc
index 1f1051d41..29db9c023 100644
--- a/absl/strings/str_cat_test.cc
+++ b/absl/strings/str_cat_test.cc
@@ -408,6 +408,20 @@ TEST(StrCat, VectorBoolReferenceTypes) {
   EXPECT_EQ(result, "1010");
 }
 
+// Passing nullptr to memcpy is undefined behavior and this test
+// provides coverage of codepaths that handle empty strings with nullptrs.
+TEST(StrCat, AvoidsMemcpyWithNullptr) {
+  EXPECT_EQ(absl::StrCat(42, absl::string_view{}), "42");
+
+  // Cover CatPieces code.
+  EXPECT_EQ(absl::StrCat(1, 2, 3, 4, 5, absl::string_view{}), "12345");
+
+  // Cover AppendPieces.
+  std::string result;
+  absl::StrAppend(&result, 1, 2, 3, 4, 5, absl::string_view{});
+  EXPECT_EQ(result, "12345");
+}
+
 #ifdef GTEST_HAS_DEATH_TEST
 TEST(StrAppend, Death) {
   std::string s = "self";