AK+LibWeb: Make StringBase::bytes() lvalue-only

Disallow calling `StringBase::bytes()` on temporaries to avoid returning
`ReadonlyBytes` that outlive the underlying string.

With this change, we catch a real UAF:
`load_result.data = maybe_response.release_value().bytes();`
All other updated call sites were already safe, they just needed to use
an intermediate named variable to satisfy the new lvalue-only
requirement.
This commit is contained in:
Aliaksandr Kalenik 2025-11-25 18:06:48 +01:00 committed by Tim Flynn
parent d1f34efa64
commit 69cede4a0f
Notes: github-actions[bot] 2025-11-25 18:03:33 +00:00
8 changed files with 31 additions and 16 deletions

View file

@ -71,7 +71,8 @@ public:
// Returns the underlying UTF-8 encoded bytes.
// NOTE: There is no guarantee about null-termination.
[[nodiscard]] ReadonlyBytes bytes() const LIFETIME_BOUND;
[[nodiscard]] ReadonlyBytes bytes() const&& = delete;
[[nodiscard]] ReadonlyBytes bytes() const& LIFETIME_BOUND;
[[nodiscard]] u32 hash() const;
[[nodiscard]] size_t byte_count() const;
[[nodiscard]] ALWAYS_INLINE size_t length_in_code_units() const { return byte_count(); }
@ -204,7 +205,7 @@ inline size_t ShortString::byte_count() const
return byte_count_and_short_string_flag >> StringBase::SHORT_STRING_BYTE_COUNT_SHIFT_COUNT;
}
inline ReadonlyBytes StringBase::bytes() const
inline ReadonlyBytes StringBase::bytes() const&
{
if (is_short_string())
return m_impl.short_string.bytes();

View file

@ -103,7 +103,8 @@ WebIDL::ExceptionOr<Infrastructure::BodyWithType> extract_body(JS::Realm& realm,
source = serialized_form_data.serialized_data;
// FIXME: Set length to unclear, see html/6424 for improving this.
// Set type to `multipart/form-data; boundary=`, followed by the multipart/form-data boundary string generated by the multipart/form-data encoding algorithm.
type = MUST(ByteBuffer::copy(MUST(String::formatted("multipart/form-data; boundary={}", serialized_form_data.boundary)).bytes()));
auto type_string = MUST(String::formatted("multipart/form-data; boundary={}", serialized_form_data.boundary));
type = MUST(ByteBuffer::copy(type_string.bytes()));
return {};
},
[&](GC::Root<DOMURL::URLSearchParams> const& url_search_params) -> WebIDL::ExceptionOr<void> {

View file

@ -1795,8 +1795,10 @@ GC::Ref<PendingResponse> http_network_or_cache_fetch(JS::Realm& realm, Infrastru
// 8. If contentLength is non-null, then set contentLengthHeaderValue to contentLength, serialized and
// isomorphic encoded.
if (content_length.has_value())
content_length_header_value = MUST(ByteBuffer::copy(String::number(*content_length).bytes()));
if (content_length.has_value()) {
auto content_length_string = String::number(*content_length);
content_length_header_value = MUST(ByteBuffer::copy(content_length_string.bytes()));
}
// 9. If contentLengthHeaderValue is non-null, then append (`Content-Length`, contentLengthHeaderValue) to
// httpRequests header list.

View file

@ -215,7 +215,8 @@ String Request::serialize_origin() const
ByteBuffer Request::byte_serialize_origin() const
{
// Byte-serializing a request origin, given a request request, is to return the result of serializing a request origin with request, isomorphic encoded.
return MUST(ByteBuffer::copy(serialize_origin().bytes()));
auto serialized_origin = serialize_origin();
return MUST(ByteBuffer::copy(serialized_origin.bytes()));
}
// https://fetch.spec.whatwg.org/#concept-request-clone
@ -285,14 +286,17 @@ void Request::add_range_header(u64 first, Optional<u64> const& last)
auto range_value = MUST(ByteBuffer::copy("bytes"sv.bytes()));
// 3. Serialize and isomorphic encode first, and append the result to rangeValue.
range_value.append(String::number(first).bytes());
auto serialized_first = String::number(first);
range_value.append(serialized_first.bytes());
// 4. Append 0x2D (-) to rangeValue.
range_value.append('-');
// 5. If last is given, then serialize and isomorphic encode it, and append the result to rangeValue.
if (last.has_value())
range_value.append(String::number(*last).bytes());
if (last.has_value()) {
auto serialized_last = String::number(*last);
range_value.append(serialized_last.bytes());
}
// 6. Append (`Range`, rangeValue) to requests header list.
auto header = Header {

View file

@ -861,7 +861,8 @@ ErrorOr<void> HTMLFormElement::submit_as_entity_body(URL::URL parsed_action, Vec
auto pairs = TRY(convert_to_list_of_name_value_pairs(entry_list));
// 2. Let body be the result of running the application/x-www-form-urlencoded serializer with pairs and encoding.
body = TRY(ByteBuffer::copy(url_encode(pairs, encoding).bytes()));
auto query = url_encode(pairs, encoding);
body = TRY(ByteBuffer::copy(query.bytes()));
// 3. Set body to the result of encoding body.
// NOTE: `encoding` refers to `UTF-8 encode`, which body already is encoded as because it uses AK::String.
@ -888,7 +889,8 @@ ErrorOr<void> HTMLFormElement::submit_as_entity_body(URL::URL parsed_action, Vec
auto pairs = TRY(convert_to_list_of_name_value_pairs(entry_list));
// 2. Let body be the result of running the text/plain encoding algorithm with pairs.
body = TRY(ByteBuffer::copy(TRY(plain_text_encode(pairs)).bytes()));
auto serialized_body = TRY(plain_text_encode(pairs));
body = TRY(ByteBuffer::copy(serialized_body.bytes()));
// FIXME: 3. Set body to the result of encoding body using encoding.

View file

@ -326,7 +326,7 @@ void ResourceLoader::handle_resource_load_request(LoadRequest const& request, Re
}
FileLoadResult load_result;
load_result.data = maybe_response.release_value().bytes();
load_result.data = maybe_response.value().bytes();
load_result.response_headers.set("Content-Type"sv, "text/html"sv);
on_resource(load_result);
return;

View file

@ -108,8 +108,11 @@ Optional<URL::URL> determine_requests_referrer(Fetch::Infrastructure::Request co
// 6. If the result of serializing referrerURL is a string whose length is greater than 4096, set referrerURL to
// referrerOrigin.
if (referrer_url.has_value() && referrer_url.value().serialize().bytes().size() > 4096)
if (referrer_url.has_value()) {
auto serialized_referrer_url = referrer_url.value().serialize();
if (serialized_referrer_url.bytes().size() > 4096)
referrer_url = referrer_origin;
}
// 7. The user agent MAY alter referrerURL or referrerOrigin at this point to enforce arbitrary policy
// considerations in the interests of minimizing data leakage. For example, the user agent could strip the URL

View file

@ -573,7 +573,8 @@ WebIDL::ExceptionOr<void> XMLHttpRequest::send(Optional<DocumentOrXMLHttpRequest
// 2. If body is a Document, then set thiss request body to body, serialized, converted, and UTF-8 encoded.
if (body->has<GC::Root<DOM::Document>>()) {
auto string_serialized_document = TRY(body->get<GC::Root<DOM::Document>>().cell()->serialize_fragment(HTML::RequireWellFormed::No));
m_request_body = Fetch::Infrastructure::byte_sequence_as_body(realm, string_serialized_document.to_utf8().bytes());
auto string_serialized_document_utf8 = string_serialized_document.to_utf8();
m_request_body = Fetch::Infrastructure::byte_sequence_as_body(realm, string_serialized_document_utf8.bytes());
}
// 3. Otherwise:
else {
@ -1014,7 +1015,8 @@ String XMLHttpRequest::get_all_response_headers() const
output.append(0x3A); // ':'
output.append(0x20); // ' '
// FIXME: The spec does not mention isomorphic decode. Spec bug?
output.append(Infra::isomorphic_decode(header.value).bytes());
auto decoder_header = Infra::isomorphic_decode(header.value);
output.append(decoder_header.bytes());
output.append(0x0D); // '\r'
output.append(0x0A); // '\n'
}