mirror of
https://github.com/RGBCube/serenity
synced 2025-07-24 22:07:34 +00:00
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".
This commit is contained in:
parent
cc35bab143
commit
7e6341587b
7 changed files with 29 additions and 14 deletions
10
AK/Error.h
10
AK/Error.h
|
@ -38,6 +38,16 @@ public:
|
||||||
return Error(syscall_name, rc);
|
return Error(syscall_name, rc);
|
||||||
}
|
}
|
||||||
[[nodiscard]] static Error from_string_view(StringView string_literal) { return Error(string_literal); }
|
[[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
|
#endif
|
||||||
|
|
||||||
[[nodiscard]] static Error copy(Error const& error)
|
[[nodiscard]] static Error copy(Error const& error)
|
||||||
|
|
|
@ -372,7 +372,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
|
||||||
auto file_or_error = Core::File::open(path, Core::File::OpenMode::Read);
|
auto file_or_error = Core::File::open(path, Core::File::OpenMode::Read);
|
||||||
if (file_or_error.is_error()) {
|
if (file_or_error.is_error()) {
|
||||||
s_error_string = DeprecatedString::formatted("Unable to open file {}", path);
|
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 file = file_or_error.release_value();
|
||||||
auto string = MUST(file->read_until_eof());
|
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");
|
auto maybe_exposed = interface.extended_attributes.get("Exposed");
|
||||||
if (!maybe_exposed.has_value()) {
|
if (!maybe_exposed.has_value()) {
|
||||||
s_error_string = DeprecatedString::formatted("Interface {} is missing extended attribute Exposed", interface.name);
|
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();
|
auto exposed = maybe_exposed.value().trim_whitespace();
|
||||||
if (exposed == "*"sv)
|
if (exposed == "*"sv)
|
||||||
|
@ -459,18 +459,18 @@ static ErrorOr<ExposedTo> parse_exposure_set(IDL::Interface& interface)
|
||||||
whom |= ExposedTo::AudioWorklet;
|
whom |= ExposedTo::AudioWorklet;
|
||||||
} else {
|
} else {
|
||||||
s_error_string = DeprecatedString::formatted("Unknown Exposed attribute candidate {} in {} in {}", candidate, exposed, interface.name);
|
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) {
|
if (whom == ExposedTo::Nobody) {
|
||||||
s_error_string = DeprecatedString::formatted("Unknown Exposed attribute {} in {}", exposed, interface.name);
|
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;
|
return whom;
|
||||||
}
|
}
|
||||||
|
|
||||||
s_error_string = DeprecatedString::formatted("Unknown Exposed attribute {} in {}", exposed, interface.name);
|
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)
|
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)
|
||||||
|
|
|
@ -1801,7 +1801,7 @@ ErrorOr<NonnullRefPtr<GUI::Action>> HackStudioWidget::create_open_project_config
|
||||||
|
|
||||||
if (FileSystem::exists(parent_directory) && !FileSystem::is_directory(parent_directory)) {
|
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);
|
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);
|
auto maybe_error = Core::System::mkdir(LexicalPath::absolute_path(m_project->root_path(), parent_directory), 0755);
|
||||||
|
|
|
@ -89,7 +89,7 @@ ErrorOr<void> JsonArrayModel::set(int row, Vector<JsonValue>&& fields)
|
||||||
VERIFY(fields.size() == m_fields.size());
|
VERIFY(fields.size() == m_fields.size());
|
||||||
|
|
||||||
if ((size_t)row >= m_array.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;
|
JsonObject obj;
|
||||||
for (size_t i = 0; i < m_fields.size(); ++i) {
|
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)
|
ErrorOr<void> JsonArrayModel::remove(int row)
|
||||||
{
|
{
|
||||||
if ((size_t)row >= m_array.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);
|
||||||
|
|
||||||
JsonArray new_array;
|
JsonArray new_array;
|
||||||
for (size_t i = 0; i < m_array.size(); ++i)
|
for (size_t i = 0; i < m_array.size(); ++i)
|
||||||
|
|
|
@ -12,11 +12,16 @@
|
||||||
#include <LibCrypto/ASN1/DER.h>
|
#include <LibCrypto/ASN1/DER.h>
|
||||||
#include <LibCrypto/ASN1/PEM.h>
|
#include <LibCrypto/ASN1/PEM.h>
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
static String s_error_string;
|
||||||
|
}
|
||||||
|
|
||||||
namespace TLS {
|
namespace TLS {
|
||||||
|
|
||||||
#define ERROR_WITH_SCOPE(error) \
|
#define ERROR_WITH_SCOPE(error) \
|
||||||
do { \
|
do { \
|
||||||
return Error::from_string_view(TRY(String::formatted("{}: {}", current_scope, error))); \
|
s_error_string = TRY(String::formatted("{}: {}", current_scope, error)); \
|
||||||
|
return Error::from_string_view(s_error_string.bytes_as_string_view()); \
|
||||||
} while (0)
|
} while (0)
|
||||||
|
|
||||||
#define ENTER_TYPED_SCOPE(tag_kind_name, scope) \
|
#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)
|
else if (curve == curve_prime256)
|
||||||
return SupportedGroup::SECP256R1;
|
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)
|
static ErrorOr<Crypto::UnsignedBigInteger> parse_version(Crypto::ASN1::Decoder& decoder, Vector<StringView> current_scope)
|
||||||
|
|
|
@ -33,7 +33,7 @@ struct Context {
|
||||||
|
|
||||||
struct ValidationError : public Error {
|
struct ValidationError : public Error {
|
||||||
ValidationError(DeprecatedString error)
|
ValidationError(DeprecatedString error)
|
||||||
: Error(Error::from_string_view(error))
|
: Error(Error::from_string_view(error.view()))
|
||||||
, error_string(move(error))
|
, error_string(move(error))
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
|
@ -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())
|
if (auto numeric_weekday = AK::StringUtils::convert_to_int(weekday_name); numeric_weekday.has_value())
|
||||||
return numeric_weekday.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()
|
static ErrorOr<int> default_weekday_start()
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue