Firestore: string_format.h: fix use-after-free bug when internally formatting strings

Firestore/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fixed use-after-free bug when internally formatting strings. (#14306)
+
 # 11.6.0
 - [fixed] Add conditional `Sendable` conformance so `ServerTimestamp<T>` is
   `Sendable` if `T` is `Sendable`. (#14042)

Firestore/core/src/util/string_format.h

@@ -65,8 +65,11 @@ struct FormatChoice<5> {};
 class FormatArg : public absl::AlphaNum {
  public:
   template <typename T>
-  FormatArg(T&& value)  // NOLINT(runtime/explicit)
-      : FormatArg{std::forward<T>(value), internal::FormatChoice<0>{}} {
+  FormatArg(T&& value,
+            absl::strings_internal::StringifySink&& sink =
+                {})  // NOLINT(runtime/explicit)
+      : FormatArg{std::forward<T>(value), std::move(sink),
+                  internal::FormatChoice<0>{}} {
   }
 
  private:
@@ -79,7 +82,9 @@ class FormatArg : public absl::AlphaNum {
    */
   template <typename T,
             typename = typename std::enable_if<std::is_same<bool, T>{}>::type>
-  FormatArg(T bool_value, internal::FormatChoice<0>)
+  FormatArg(T bool_value,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<0>)
       : AlphaNum(bool_value ? "true" : "false") {
   }
 
@@ -90,15 +95,19 @@ class FormatArg : public absl::AlphaNum {
   template <
       typename T,
       typename = typename std::enable_if<objc::is_objc_pointer<T>{}>::type>
-  FormatArg(T object, internal::FormatChoice<1>)
+  FormatArg(T object,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<1>)
       : AlphaNum(MakeStringView([object description])) {
   }
 
   /**
    * Creates a FormatArg from any Objective-C Class type. Objective-C Class
    * types are a special struct that aren't of a type derived from NSObject.
    */
-  FormatArg(Class object, internal::FormatChoice<1>)
+  FormatArg(Class object,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<1>)
       : AlphaNum(MakeStringView(NSStringFromClass(object))) {
   }
 #endif
@@ -108,15 +117,20 @@ class FormatArg : public absl::AlphaNum {
    * handled specially to avoid ambiguity with generic pointers, which are
    * handled differently.
    */
-  FormatArg(std::nullptr_t, internal::FormatChoice<2>) : AlphaNum("null") {
+  FormatArg(std::nullptr_t,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<2>)
+      : AlphaNum("null") {
   }
 
   /**
    * Creates a FormatArg from a character string literal. This is
    * handled specially to avoid ambiguity with generic pointers, which are
    * handled differently.
    */
-  FormatArg(const char* string_value, internal::FormatChoice<3>)
+  FormatArg(const char* string_value,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<3>)
       : AlphaNum(string_value == nullptr ? "null" : string_value) {
   }
 
@@ -125,16 +139,21 @@ class FormatArg : public absl::AlphaNum {
    * hexadecimal integer literal.
    */
   template <typename T>
-  FormatArg(T* pointer_value, internal::FormatChoice<4>)
-      : AlphaNum(absl::Hex(reinterpret_cast<uintptr_t>(pointer_value))) {
+  FormatArg(T* pointer_value,
+            absl::strings_internal::StringifySink&& sink,
+            internal::FormatChoice<4>)
+      : AlphaNum(absl::Hex(reinterpret_cast<uintptr_t>(pointer_value)),
+                 std::move(sink)) {
   }
 
   /**
    * As a final fallback, creates a FormatArg from any type of value that
    * absl::AlphaNum accepts.
    */
   template <typename T>
-  FormatArg(T&& value, internal::FormatChoice<5>)
+  FormatArg(T&& value,
+            absl::strings_internal::StringifySink&&,
+            internal::FormatChoice<5>)
       : AlphaNum(std::forward<T>(value)) {
   }
 };

Firestore/core/test/unit/util/string_format_test.cc

@@ -17,6 +17,7 @@
 #include "Firestore/core/src/util/string_format.h"
 
 #include "absl/strings/string_view.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace firebase {
@@ -72,14 +73,18 @@ TEST(StringFormatTest, Bool) {
   EXPECT_EQ("Hello false", StringFormat("Hello %s", false));
 }
 
-TEST(StringFormatTest, Pointer) {
-  // pointers implicitly convert to bool. Make sure this doesn't happen in
-  // this API.
-  int value = 4;
-  EXPECT_NE("Hello true", StringFormat("Hello %s", &value));
+TEST(StringFormatTest, NullPointer) {
+  // pointers implicitly convert to bool. Make sure this doesn't happen here.
   EXPECT_EQ("Hello null", StringFormat("Hello %s", nullptr));
 }
 
+TEST(StringFormatTest, NonNullPointer) {
+  // pointers implicitly convert to bool. Make sure this doesn't happen here.
+  int value = 4;
+  EXPECT_THAT(StringFormat("Hello %s", &value),
+              testing::MatchesRegex("Hello (0x)?[0123456789abcdefABCDEF]+"));
+}
+
 TEST(StringFormatTest, Mixed) {
   EXPECT_EQ("string=World, bool=true, int=42, float=1.5",
             StringFormat("string=%s, bool=%s, int=%s, float=%s", "World", true,