AK+Everywhere: Disallow Error::from_string_view(FooString)

That pattern seems to show up a lot in code written by people that
aren't intimately familiar with the lifetime model of Error and Strings.
This commit makes the compiler detect it and present a more helpful
diagnostic than "garbage string at runtime".

7 files changed, 29 insertions, 14 deletions

diff --git a/AK/Error.h b/AK/Error.h
index 02eccaf5ca..1405f07a3f 100644
--- a/AK/Error.h
+++ b/AK/Error.h
@@ -38,6 +38,16 @@ public:
         return Error(syscall_name, rc);
     }
     [[nodiscard]] static Error from_string_view(StringView string_literal) { return Error(string_literal); }
+
+    template<OneOf<DeprecatedString, DeprecatedFlyString, String, FlyString> T>
+    static Error from_string_view(T)
+    {
+        // `Error::from_string_view(DeprecatedString::formatted(...))` is a somewhat common mistake, which leads to a UAF situation.
+        // If your string outlives this error and _isn't_ a temporary being passed to this function, explicitly call .view() on it to resolve to the StringView overload.
+        static_assert(DependentFalse<T>, "Error::from_string_view(String) is almost always a use-after-free");
+        VERIFY_NOT_REACHED();
+    }
+
 #endif
 
     [[nodiscard]] static Error copy(Error const& error)
diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp
index 3cecec74d7..4a016c55ce 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp
@@ -372,7 +372,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         auto file_or_error = Core::File::open(path, Core::File::OpenMode::Read);
         if (file_or_error.is_error()) {
             s_error_string = DeprecatedString::formatted("Unable to open file {}", path);
-            return Error::from_string_view(s_error_string);
+            return Error::from_string_view(s_error_string.view());
         }
         auto file = file_or_error.release_value();
         auto string = MUST(file->read_until_eof());
@@ -429,7 +429,7 @@ static ErrorOr<ExposedTo> parse_exposure_set(IDL::Interface& interface)
     auto maybe_exposed = interface.extended_attributes.get("Exposed");
     if (!maybe_exposed.has_value()) {
         s_error_string = DeprecatedString::formatted("Interface {} is missing extended attribute Exposed", interface.name);
-        return Error::from_string_view(s_error_string);
+        return Error::from_string_view(s_error_string.view());
     }
     auto exposed = maybe_exposed.value().trim_whitespace();
     if (exposed == "*"sv)
@@ -459,18 +459,18 @@ static ErrorOr<ExposedTo> parse_exposure_set(IDL::Interface& interface)
                 whom |= ExposedTo::AudioWorklet;
             } else {
                 s_error_string = DeprecatedString::formatted("Unknown Exposed attribute candidate {} in {} in {}", candidate, exposed, interface.name);
-                return Error::from_string_view(s_error_string);
+                return Error::from_string_view(s_error_string.view());
             }
         }
         if (whom == ExposedTo::Nobody) {
             s_error_string = DeprecatedString::formatted("Unknown Exposed attribute {} in {}", exposed, interface.name);
-            return Error::from_string_view(s_error_string);
+            return Error::from_string_view(s_error_string.view());
         }
         return whom;
     }
 
     s_error_string = DeprecatedString::formatted("Unknown Exposed attribute {} in {}", exposed, interface.name);
-    return Error::from_string_view(s_error_string);
+    return Error::from_string_view(s_error_string.view());
 }
 
 ErrorOr<void> add_to_interface_sets(IDL::Interface& interface, Vector<IDL::Interface&>& intrinsics, Vector<IDL::Interface&>& window_exposed, Vector<IDL::Interface&>& dedicated_worker_exposed, Vector<IDL::Interface&>& shared_worker_exposed)
diff --git a/Userland/DevTools/HackStudio/HackStudioWidget.cpp b/Userland/DevTools/HackStudio/HackStudioWidget.cpp
index 02fcc289c5..97b45ccc25 100644
--- a/Userland/DevTools/HackStudio/HackStudioWidget.cpp
+++ b/Userland/DevTools/HackStudio/HackStudioWidget.cpp
@@ -1801,7 +1801,7 @@ ErrorOr<NonnullRefPtr<GUI::Action>> HackStudioWidget::create_open_project_config
 
             if (FileSystem::exists(parent_directory) && !FileSystem::is_directory(parent_directory)) {
                 formatted_error_string_holder = DeprecatedString::formatted("Cannot create the '{}' directory because there is already a file with that name", parent_directory);
-                return Error::from_string_view(formatted_error_string_holder);
+                return Error::from_string_view(formatted_error_string_holder.view());
             }
 
             auto maybe_error = Core::System::mkdir(LexicalPath::absolute_path(m_project->root_path(), parent_directory), 0755);
diff --git a/Userland/Libraries/LibGUI/JsonArrayModel.cpp b/Userland/Libraries/LibGUI/JsonArrayModel.cpp
index 97733033d0..b5baec6a60 100644
--- a/Userland/Libraries/LibGUI/JsonArrayModel.cpp
+++ b/Userland/Libraries/LibGUI/JsonArrayModel.cpp
@@ -89,7 +89,7 @@ ErrorOr<void> JsonArrayModel::set(int row, Vector<JsonValue>&& fields)
     VERIFY(fields.size() == m_fields.size());
 
     if ((size_t)row >= m_array.size())
-        return Error::from_string_view(TRY(String::formatted("Row out of bounds: {} >= {}", row, m_array.size())));
+        return Error::from_string_view("Row out of bounds"sv);
 
     JsonObject obj;
     for (size_t i = 0; i < m_fields.size(); ++i) {
@@ -106,7 +106,7 @@ ErrorOr<void> JsonArrayModel::set(int row, Vector<JsonValue>&& fields)
 ErrorOr<void> JsonArrayModel::remove(int row)
 {
     if ((size_t)row >= m_array.size())
-        return Error::from_string_view(TRY(String::formatted("Row out of bounds: {} >= {}", row, m_array.size())));
+        return Error::from_string_view("Row out of bounds"sv);
 
     JsonArray new_array;
     for (size_t i = 0; i < m_array.size(); ++i)
diff --git a/Userland/Libraries/LibTLS/Certificate.cpp b/Userland/Libraries/LibTLS/Certificate.cpp
index 839538cc74..dbd3f9cf30 100644
--- a/Userland/Libraries/LibTLS/Certificate.cpp
+++ b/Userland/Libraries/LibTLS/Certificate.cpp
@@ -12,11 +12,16 @@
 #include <LibCrypto/ASN1/DER.h>
 #include <LibCrypto/ASN1/PEM.h>
 
+namespace {
+static String s_error_string;
+}
+
 namespace TLS {
 
-#define ERROR_WITH_SCOPE(error)                                                                 \
-    do {                                                                                        \
-        return Error::from_string_view(TRY(String::formatted("{}: {}", current_scope, error))); \
+#define ERROR_WITH_SCOPE(error)                                                  \
+    do {                                                                         \
+        s_error_string = TRY(String::formatted("{}: {}", current_scope, error)); \
+        return Error::from_string_view(s_error_string.bytes_as_string_view());   \
     } while (0)
 
 #define ENTER_TYPED_SCOPE(tag_kind_name, scope)                                                                                                               \
@@ -77,7 +82,7 @@ static ErrorOr<SupportedGroup> oid_to_curve(Vector<int> curve)
     else if (curve == curve_prime256)
         return SupportedGroup::SECP256R1;
 
-    return Error::from_string_view(TRY(String::formatted("Unknown curve oid {}", curve)));
+    return Error::from_string_view("Unknown curve oid"sv);
 }
 
 static ErrorOr<Crypto::UnsignedBigInteger> parse_version(Crypto::ASN1::Decoder& decoder, Vector<StringView> current_scope)
diff --git a/Userland/Libraries/LibWasm/AbstractMachine/Validator.h b/Userland/Libraries/LibWasm/AbstractMachine/Validator.h
index 3ff7775313..809ef2cd9b 100644
--- a/Userland/Libraries/LibWasm/AbstractMachine/Validator.h
+++ b/Userland/Libraries/LibWasm/AbstractMachine/Validator.h
@@ -33,7 +33,7 @@ struct Context {
 
 struct ValidationError : public Error {
     ValidationError(DeprecatedString error)
-        : Error(Error::from_string_view(error))
+        : Error(Error::from_string_view(error.view()))
         , error_string(move(error))
     {
     }
diff --git a/Userland/Utilities/cal.cpp b/Userland/Utilities/cal.cpp
index 39ca1ebd4c..f4d3992d11 100644
--- a/Userland/Utilities/cal.cpp
+++ b/Userland/Utilities/cal.cpp
@@ -45,7 +45,7 @@ static ErrorOr<int> weekday_index(StringView weekday_name)
     if (auto numeric_weekday = AK::StringUtils::convert_to_int(weekday_name); numeric_weekday.has_value())
         return numeric_weekday.value();
 
-    return Error::from_string_view(TRY(String::formatted("Unknown weekday name: '{}'", weekday_name)));
+    return Error::from_string_view("Unknown weekday name"sv);
 }
 
 static ErrorOr<int> default_weekday_start()