Fix UAF in FontCache

Fixes a rare crash related to font caching.
Upstream issue: https://issues.chromium.org/issues/342778288
Its been only fixed for ASAN builds for some reason.

This change backports the fix: https://crrev.com/c/5629253
and extends the ASAN guards with IS_QTWEBENGINE.

Change-Id: I0fb7348ef97882fed199d1432b3a2543804e8de5
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/592190
Reviewed-by: Peter Varga <[email protected]>

Author: mnegyokru

chromium/third_party/blink/renderer/platform/fonts/font_face_creation_params.h

@@ -38,6 +38,8 @@
 #include "third_party/blink/renderer/platform/wtf/text/case_folding_hash.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_hasher.h"
 
+#include <optional>
+
 namespace blink {
 
 enum FontFaceCreationType {
@@ -49,19 +51,10 @@ class FontFaceCreationParams {
   USING_FAST_MALLOC(FontFaceCreationParams);
 
  public:
-  FontFaceCreationParams()
-      : creation_type_(kCreateFontByFamily),
-        family_(AtomicString()),
-        filename_(std::string()),
-        fontconfig_interface_id_(0),
-        ttc_index_(0) {}
+  FontFaceCreationParams() : creation_type_(kCreateFontByFamily) {}
 
   explicit FontFaceCreationParams(AtomicString family)
-      : creation_type_(kCreateFontByFamily),
-        family_(family),
-        filename_(std::string()),
-        fontconfig_interface_id_(0),
-        ttc_index_(0) {
+      : creation_type_(kCreateFontByFamily), family_(family) {
 #if BUILDFLAG(IS_WIN)
     // Leading "@" in the font name enables Windows vertical flow flag for the
     // font.  Because we do vertical flow by ourselves, we don't want to use the
@@ -88,8 +81,14 @@ class FontFaceCreationParams {
   }
   const std::string& Filename() const {
     DCHECK_EQ(creation_type_, kCreateFontByFciIdAndTtcIndex);
+#if defined(ADDRESS_SANITIZER) || BUILDFLAG(IS_QTWEBENGINE)
+    DCHECK(filename_.has_value());
+    return *filename_;
+#else
     return filename_;
+#endif
   }
+
   int FontconfigInterfaceId() const {
     DCHECK_EQ(creation_type_, kCreateFontByFciIdAndTtcIndex);
     return fontconfig_interface_id_;
@@ -106,8 +105,11 @@ class FontFaceCreationParams {
       // encoding and endianness. However, since the hash is not transferred
       // over a network or permanently stored and only used for the runtime of
       // Chromium, this is not a concern.
-      hasher.AddCharacters(reinterpret_cast<const LChar*>(filename_.data()),
-                           static_cast<unsigned>(filename_.length()));
+      if (HasFilename()) {
+        const auto& filename = Filename();
+        hasher.AddCharacters(reinterpret_cast<const LChar*>(filename.data()),
+                             static_cast<unsigned>(filename.length()));
+      }
       hasher.AddCharacters(reinterpret_cast<const LChar*>(&ttc_index_),
                            sizeof(ttc_index_));
       hasher.AddCharacters(
@@ -121,17 +123,56 @@ class FontFaceCreationParams {
   bool operator==(const FontFaceCreationParams& other) const {
     return creation_type_ == other.creation_type_ &&
            DeprecatedEqualIgnoringCase(family_, other.family_) &&
-           filename_ == other.filename_ &&
+           FilenameEqual(other) &&
            fontconfig_interface_id_ == other.fontconfig_interface_id_ &&
            ttc_index_ == other.ttc_index_;
   }
 
  private:
   FontFaceCreationType creation_type_;
   AtomicString family_;
+
+  void SetFilename(std::string& filename) {
+#if defined(ADDRESS_SANITIZER) || BUILDFLAG(IS_QTWEBENGINE)
+    *filename_ = filename;
+#else
+    filename_ = filename;
+#endif
+  }
+
+  bool FilenameEqual(const FontFaceCreationParams& other) const {
+#if defined(ADDRESS_SANITIZER) || BUILDFLAG(IS_QTWEBENGINE)
+    if (!filename_.has_value() || !other.filename_.has_value()) {
+      return filename_.has_value() == other.filename_.has_value();
+    }
+    return *filename_ == *other.filename_;
+#else
+    return filename_ == other.filename_;
+#endif
+  }
+
+  bool HasFilename() const {
+#if defined(ADDRESS_SANITIZER) || BUILDFLAG(IS_QTWEBENGINE)
+    return filename_.has_value();
+#else
+    return true;
+#endif
+  }
+
+#if defined(ADDRESS_SANITIZER) || BUILDFLAG(IS_QTWEBENGINE)
+  // We put the `std::string` behind an optional as ASAN counter checks require
+  // that we properly call constructors and destructors for all strings. This is
+  // not the case when `FontFaceCreationParams` is used in `WTF::HashMap` as key
+  // where we also cosntruct empty and deleted values that are never properly
+  // destroyed.
+  //
+  // See crbug.com/346174906.
+  std::optional<std::string> filename_;
+#else
   std::string filename_;
-  int fontconfig_interface_id_;
-  int ttc_index_;
+#endif
+  int fontconfig_interface_id_ = 0;
+  int ttc_index_ = 0;
 };
 
 }  // namespace blink