Skip to content

Commit 82bd3d3

Browse files
Lubrsitrflynn89
Lubrsi
authored and
trflynn89
committed
LibWeb: Avoid invoking Trusted Types where avoidable
Prevents observably calling Trusted Types, which can run arbitrary JS, cause crashes due to use of MUST and allow arbitrary JS to modify internal elements.
1 parent fb9406d commit 82bd3d3

File tree

83 files changed

+406
-365
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+406
-365
lines changed

‎Libraries/LibWeb/ARIA/ARIAMixin.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class WEB_API ARIAMixin {
3434

3535
#define __ENUMERATE_ARIA_ATTRIBUTE(name, attribute) \
3636
virtual Optional<String> name() const = 0; \
37-
virtual WebIDL::ExceptionOr<void> set_##name(Optional<String> const&) = 0;
37+
virtual void set_##name(Optional<String> const&) = 0;
3838
ENUMERATE_ARIA_ATTRIBUTES
3939
#undef __ENUMERATE_ARIA_ATTRIBUTE
4040

‎Libraries/LibWeb/Bindings/AudioConstructor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,15 @@ JS::ThrowCompletionOr<GC::Ref<JS::Object>> AudioConstructor::construct(FunctionO
4949
auto audio = TRY(Bindings::throw_dom_exception_if_needed(vm, [&]() { return DOM::create_element(document, HTML::TagNames::audio, Namespace::HTML); }));
5050

5151
// 3. Set an attribute value for audio using "preload" and "auto".
52-
MUST(audio->set_attribute(HTML::AttributeNames::preload, "auto"_string));
52+
audio->set_attribute_value(HTML::AttributeNames::preload, "auto"_string);
5353

5454
auto src_value = vm.argument(0);
5555

5656
// 4. If src is given, then set an attribute value for audio using "src" and src.
5757
// (This will cause the user agent to invoke the object's resource selection algorithm before returning.)
5858
if (!src_value.is_undefined()) {
5959
auto src = TRY(src_value.to_string(vm));
60-
MUST(audio->set_attribute(HTML::AttributeNames::src, move(src)));
60+
audio->set_attribute_value(HTML::AttributeNames::src, move(src));
6161
}
6262

6363
// 5. Return audio.

‎Libraries/LibWeb/Bindings/ImageConstructor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ JS::ThrowCompletionOr<GC::Ref<JS::Object>> ImageConstructor::construct(FunctionO
5353
// 3. If width is given, then set an attribute value for img using "width" and width.
5454
if (vm.argument_count() > 0) {
5555
u32 width = TRY(vm.argument(0).to_u32(vm));
56-
MUST(image_element->set_attribute(HTML::AttributeNames::width, MUST(String::formatted("{}", width))));
56+
image_element->set_attribute_value(HTML::AttributeNames::width, MUST(String::formatted("{}", width)));
5757
}
5858

5959
// 4. If height is given, then set an attribute value for img using "height" and height.
6060
if (vm.argument_count() > 1) {
6161
u32 height = TRY(vm.argument(1).to_u32(vm));
62-
MUST(image_element->set_attribute(HTML::AttributeNames::height, MUST(String::formatted("{}", height))));
62+
image_element->set_attribute_value(HTML::AttributeNames::height, MUST(String::formatted("{}", height)));
6363
}
6464

6565
// 5. Return img.

‎Libraries/LibWeb/Bindings/OptionConstructor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,14 @@ JS::ThrowCompletionOr<GC::Ref<JS::Object>> OptionConstructor::construct(Function
6969
// 4. If value is given, then set an attribute value for option using "value" and value.
7070
if (!vm.argument(1).is_undefined()) {
7171
auto value = TRY(vm.argument(1).to_string(vm));
72-
MUST(option_element->set_attribute(HTML::AttributeNames::value, value));
72+
option_element->set_attribute_value(HTML::AttributeNames::value, value);
7373
}
7474

7575
// 5. If defaultSelected is true, then set an attribute value for option using "selected" and the empty string.
7676
if (vm.argument_count() > 2) {
7777
auto default_selected = vm.argument(2).to_boolean();
7878
if (default_selected) {
79-
MUST(option_element->set_attribute(HTML::AttributeNames::selected, String {}));
79+
option_element->set_attribute_value(HTML::AttributeNames::selected, String {});
8080
}
8181
}
8282

‎Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ void CSSStyleDeclaration::update_style_attribute()
4848
set_is_updating(true);
4949

5050
// 5. Set an attribute value for owner node using "style" and the result of serializing declaration block.
51-
MUST(owner_node()->element().set_attribute(HTML::AttributeNames::style, serialized()));
51+
owner_node()->element().set_attribute_value(HTML::AttributeNames::style, serialized());
5252

5353
// 6. Unset declaration block’s updating flag.
5454
set_is_updating(false);

‎Libraries/LibWeb/DOM/DOMTokenList.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ void DOMTokenList::set_value(String const& value)
258258
if (!associated_element)
259259
return;
260260

261-
MUST(associated_element->set_attribute(m_associated_attribute, value));
261+
associated_element->set_attribute_value(m_associated_attribute, value);
262262
}
263263

264264
WebIDL::ExceptionOr<void> DOMTokenList::validate_token(StringView token) const
@@ -294,7 +294,7 @@ void DOMTokenList::run_update_steps()
294294
return;
295295

296296
// 2. Set an attribute value for the associated element using associated attribute’s local name and the result of running the ordered set serializer for token set.
297-
MUST(associated_element->set_attribute(m_associated_attribute, serialize_ordered_set()));
297+
associated_element->set_attribute_value(m_associated_attribute, serialize_ordered_set());
298298
}
299299

300300
Optional<JS::Value> DOMTokenList::item_value(size_t index) const

‎Libraries/LibWeb/DOM/Document.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3189,7 +3189,7 @@ String Document::fg_color() const
31893189
void Document::set_fg_color(String const& value)
31903190
{
31913191
if (auto* body_element = body(); body_element && !is<HTML::HTMLFrameSetElement>(*body_element))
3192-
MUST(body_element->set_attribute(HTML::AttributeNames::text, value));
3192+
body_element->set_attribute_value(HTML::AttributeNames::text, value);
31933193
}
31943194

31953195
String Document::link_color() const
@@ -3202,7 +3202,7 @@ String Document::link_color() const
32023202
void Document::set_link_color(String const& value)
32033203
{
32043204
if (auto* body_element = body(); body_element && !is<HTML::HTMLFrameSetElement>(*body_element))
3205-
MUST(body_element->set_attribute(HTML::AttributeNames::link, value));
3205+
body_element->set_attribute_value(HTML::AttributeNames::link, value);
32063206
}
32073207

32083208
String Document::vlink_color() const
@@ -3215,7 +3215,7 @@ String Document::vlink_color() const
32153215
void Document::set_vlink_color(String const& value)
32163216
{
32173217
if (auto* body_element = body(); body_element && !is<HTML::HTMLFrameSetElement>(*body_element))
3218-
MUST(body_element->set_attribute(HTML::AttributeNames::vlink, value));
3218+
body_element->set_attribute_value(HTML::AttributeNames::vlink, value);
32193219
}
32203220

32213221
String Document::alink_color() const
@@ -3228,7 +3228,7 @@ String Document::alink_color() const
32283228
void Document::set_alink_color(String const& value)
32293229
{
32303230
if (auto* body_element = body(); body_element && !is<HTML::HTMLFrameSetElement>(*body_element))
3231-
MUST(body_element->set_attribute(HTML::AttributeNames::alink, value));
3231+
body_element->set_attribute_value(HTML::AttributeNames::alink, value);
32323232
}
32333233

32343234
String Document::bg_color() const
@@ -3241,7 +3241,7 @@ String Document::bg_color() const
32413241
void Document::set_bg_color(String const& value)
32423242
{
32433243
if (auto* body_element = body(); body_element && !is<HTML::HTMLFrameSetElement>(*body_element))
3244-
MUST(body_element->set_attribute(HTML::AttributeNames::bgcolor, value));
3244+
body_element->set_attribute_value(HTML::AttributeNames::bgcolor, value);
32453245
}
32463246

32473247
String Document::dump_dom_tree_as_json() const

‎Libraries/LibWeb/DOM/DocumentLoading.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ static WebIDL::ExceptionOr<GC::Ref<DOM::Document>> load_media_document(HTML::Nav
296296
};
297297

298298
auto style_element = TRY(DOM::create_element(document, HTML::TagNames::style, Namespace::HTML));
299-
MUST(style_element->set_text_content(R"~~~(
299+
style_element->string_replace_all(R"~~~(
300300
:root {
301301
background-color: #222;
302302
}
@@ -310,29 +310,29 @@ static WebIDL::ExceptionOr<GC::Ref<DOM::Document>> load_media_document(HTML::Nav
310310
img {
311311
background-color: #fff;
312312
}
313-
)~~~"_utf16));
313+
)~~~"_utf16);
314314
TRY(document->head()->append_child(style_element));
315315

316316
auto url_string = document->url_string();
317317
if (type.is_image()) {
318318
auto img_element = TRY(DOM::create_element(document, HTML::TagNames::img, Namespace::HTML));
319-
TRY(img_element->set_attribute(HTML::AttributeNames::src, url_string));
319+
img_element->set_attribute_value(HTML::AttributeNames::src, url_string);
320320
TRY(document->body()->append_child(img_element));
321321
TRY(insert_title(document, url_string));
322322

323323
} else if (type.type() == "video"sv) {
324324
auto video_element = TRY(DOM::create_element(document, HTML::TagNames::video, Namespace::HTML));
325-
TRY(video_element->set_attribute(HTML::AttributeNames::src, url_string));
326-
TRY(video_element->set_attribute(HTML::AttributeNames::autoplay, String {}));
327-
TRY(video_element->set_attribute(HTML::AttributeNames::controls, String {}));
325+
video_element->set_attribute_value(HTML::AttributeNames::src, url_string);
326+
video_element->set_attribute_value(HTML::AttributeNames::autoplay, String {});
327+
video_element->set_attribute_value(HTML::AttributeNames::controls, String {});
328328
TRY(document->body()->append_child(video_element));
329329
TRY(insert_title(document, url_string));
330330

331331
} else if (type.type() == "audio"sv) {
332332
auto audio_element = TRY(DOM::create_element(document, HTML::TagNames::audio, Namespace::HTML));
333-
TRY(audio_element->set_attribute(HTML::AttributeNames::src, url_string));
334-
TRY(audio_element->set_attribute(HTML::AttributeNames::autoplay, String {}));
335-
TRY(audio_element->set_attribute(HTML::AttributeNames::controls, String {}));
333+
audio_element->set_attribute_value(HTML::AttributeNames::src, url_string);
334+
audio_element->set_attribute_value(HTML::AttributeNames::autoplay, String {});
335+
audio_element->set_attribute_value(HTML::AttributeNames::controls, String {});
336336
TRY(document->body()->append_child(audio_element));
337337
TRY(insert_title(document, url_string));
338338

‎Libraries/LibWeb/DOM/Element.cpp‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ GC::Ptr<Attr> Element::get_attribute_node_ns(Optional<FlyString> const& namespac
210210

211211
// FIXME: Trusted Types integration with DOM is still under review https://github.com/whatwg/dom/pull/1268
212212
// https://whatpr.org/dom/1268.html#dom-element-setattribute
213-
WebIDL::ExceptionOr<void> Element::set_attribute(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value)
213+
WebIDL::ExceptionOr<void> Element::set_attribute_for_bindings(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value)
214214
{
215215
// 1. If qualifiedName is not a valid attribute local name, then throw an "InvalidCharacterError" DOMException.
216216
if (!is_valid_attribute_local_name(qualified_name))
@@ -245,9 +245,9 @@ WebIDL::ExceptionOr<void> Element::set_attribute(FlyString qualified_name, Varia
245245

246246
// FIXME: Trusted Types integration with DOM is still under review https://github.com/whatwg/dom/pull/1268
247247
// https://whatpr.org/dom/1268.html#dom-element-setattribute
248-
WebIDL::ExceptionOr<void> Element::set_attribute(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, String> const& value)
248+
WebIDL::ExceptionOr<void> Element::set_attribute_for_bindings(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, String> const& value)
249249
{
250-
return set_attribute(move(qualified_name),
250+
return set_attribute_for_bindings(move(qualified_name),
251251
value.visit(
252252
[](auto const& trusted_type) -> Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> { return trusted_type; },
253253
[](String const& string) -> Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> { return Utf16String::from_utf8(string); }));
@@ -365,7 +365,7 @@ WebIDL::ExceptionOr<QualifiedName> validate_and_extract(JS::Realm& realm, Option
365365

366366
// FIXME: Trusted Types integration with DOM is still under review https://github.com/whatwg/dom/pull/1268
367367
// https://whatpr.org/dom/1268.html#dom-element-setattributens
368-
WebIDL::ExceptionOr<void> Element::set_attribute_ns(Optional<FlyString> const& namespace_, FlyString const& qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value)
368+
WebIDL::ExceptionOr<void> Element::set_attribute_ns_for_bindings(Optional<FlyString> const& namespace_, FlyString const& qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value)
369369
{
370370
// 1. Let (namespace, prefix, localName) be the result of validating and extracting namespace and qualifiedName given "element".
371371
auto extracted_qualified_name = TRY(validate_and_extract(realm(), namespace_, qualified_name, ValidationContext::Element));
@@ -421,14 +421,14 @@ void Element::set_attribute_value(FlyString const& local_name, String const& val
421421
}
422422

423423
// https://dom.spec.whatwg.org/#dom-element-setattributenode
424-
WebIDL::ExceptionOr<GC::Ptr<Attr>> Element::set_attribute_node(Attr& attr)
424+
WebIDL::ExceptionOr<GC::Ptr<Attr>> Element::set_attribute_node_for_bindings(Attr& attr)
425425
{
426426
// The setAttributeNode(attr) and setAttributeNodeNS(attr) methods steps are to return the result of setting an attribute given attr and this.
427427
return attributes()->set_attribute(attr);
428428
}
429429

430430
// https://dom.spec.whatwg.org/#dom-element-setattributenodens
431-
WebIDL::ExceptionOr<GC::Ptr<Attr>> Element::set_attribute_node_ns(Attr& attr)
431+
WebIDL::ExceptionOr<GC::Ptr<Attr>> Element::set_attribute_node_ns_for_bindings(Attr& attr)
432432
{
433433
// The setAttributeNode(attr) and setAttributeNodeNS(attr) methods steps are to return the result of setting an attribute given attr and this.
434434
return attributes()->set_attribute(attr);
@@ -1746,7 +1746,7 @@ i32 Element::tab_index() const
17461746
// https://html.spec.whatwg.org/multipage/interaction.html#dom-tabindex
17471747
void Element::set_tab_index(i32 tab_index)
17481748
{
1749-
MUST(set_attribute(HTML::AttributeNames::tabindex, String::number(tab_index)));
1749+
set_attribute_value(HTML::AttributeNames::tabindex, String::number(tab_index));
17501750
}
17511751

17521752
// https://drafts.csswg.org/cssom-view/#potentially-scrollable
@@ -2544,6 +2544,22 @@ ErrorOr<void> Element::scroll_into_view(Optional<Variant<bool, ScrollIntoViewOpt
25442544
return {};
25452545
}
25462546

2547+
#define __ENUMERATE_ARIA_ATTRIBUTE(name, attribute) \
2548+
Optional<String> Element::name() const \
2549+
{ \
2550+
return get_attribute(ARIA::AttributeNames::name); \
2551+
} \
2552+
\
2553+
void Element::set_##name(Optional<String> const& value) \
2554+
{ \
2555+
if (value.has_value()) \
2556+
set_attribute_value(ARIA::AttributeNames::name, *value); \
2557+
else \
2558+
remove_attribute(ARIA::AttributeNames::name); \
2559+
}
2560+
ENUMERATE_ARIA_ATTRIBUTES
2561+
#undef __ENUMERATE_ARIA_ATTRIBUTE
2562+
25472563
void Element::invalidate_style_after_attribute_change(FlyString const& attribute_name, Optional<String> const& old_value, Optional<String> const& new_value)
25482564
{
25492565
Vector<CSS::InvalidationSet::Property, 1> changed_properties;

‎Libraries/LibWeb/DOM/Element.h‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,13 @@ class WEB_API Element
149149

150150
Optional<String> lang() const;
151151

152-
WebIDL::ExceptionOr<void> set_attribute(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, String> const& value);
153-
WebIDL::ExceptionOr<void> set_attribute(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value);
152+
WebIDL::ExceptionOr<void> set_attribute_for_bindings(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value);
153+
WebIDL::ExceptionOr<void> set_attribute_for_bindings(FlyString qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, String> const& value);
154154

155-
WebIDL::ExceptionOr<void> set_attribute_ns(Optional<FlyString> const& namespace_, FlyString const& qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value);
155+
WebIDL::ExceptionOr<void> set_attribute_ns_for_bindings(Optional<FlyString> const& namespace_, FlyString const& qualified_name, Variant<GC::Root<TrustedTypes::TrustedHTML>, GC::Root<TrustedTypes::TrustedScript>, GC::Root<TrustedTypes::TrustedScriptURL>, Utf16String> const& value);
156156
void set_attribute_value(FlyString const& local_name, String const& value, Optional<FlyString> const& prefix = {}, Optional<FlyString> const& namespace_ = {});
157-
WebIDL::ExceptionOr<GC::Ptr<Attr>> set_attribute_node(Attr&);
158-
WebIDL::ExceptionOr<GC::Ptr<Attr>> set_attribute_node_ns(Attr&);
157+
WebIDL::ExceptionOr<GC::Ptr<Attr>> set_attribute_node_for_bindings(Attr&);
158+
WebIDL::ExceptionOr<GC::Ptr<Attr>> set_attribute_node_ns_for_bindings(Attr&);
159159

160160
void append_attribute(FlyString const& name, String const& value);
161161
void append_attribute(Attr&);
@@ -336,20 +336,9 @@ class WEB_API Element
336336
ErrorOr<void> scroll_into_view(Optional<Variant<bool, ScrollIntoViewOptions>> = {});
337337

338338
// https://www.w3.org/TR/wai-aria-1.2/#ARIAMixin
339-
#define __ENUMERATE_ARIA_ATTRIBUTE(name, attribute) \
340-
Optional<String> name() const override \
341-
{ \
342-
return get_attribute(ARIA::AttributeNames::name); \
343-
} \
344-
\
345-
WebIDL::ExceptionOr<void> set_##name(Optional<String> const& value) override \
346-
{ \
347-
if (value.has_value()) \
348-
TRY(set_attribute(ARIA::AttributeNames::name, *value)); \
349-
else \
350-
remove_attribute(ARIA::AttributeNames::name); \
351-
return {}; \
352-
}
339+
#define __ENUMERATE_ARIA_ATTRIBUTE(name, attribute) \
340+
virtual Optional<String> name() const override; \
341+
virtual void set_##name(Optional<String> const& value) override;
353342
ENUMERATE_ARIA_ATTRIBUTES
354343
#undef __ENUMERATE_ARIA_ATTRIBUTE
355344

0 commit comments

Comments
 (0)