Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: cd7e28cf47f603ff24c10eae96a5bf8ccbae8713
https://github.com/WebKit/WebKit/commit/cd7e28cf47f603ff24c10eae96a5bf8ccbae8713
Author: Aditya Keerthi <[email protected]>
Date: 2024-08-05 (Mon, 05 Aug 2024)
Changed paths:
A LayoutTests/fast/css/container-query-listbox-expected.html
A LayoutTests/fast/css/container-query-listbox.html
A
LayoutTests/fast/forms/select-multiple-changed-with-containment-crash-expected.txt
A LayoutTests/fast/forms/select-multiple-changed-with-containment-crash.html
M LayoutTests/platform/ios/TestExpectations
M Source/WebCore/rendering/RenderListBox.cpp
M Source/WebCore/rendering/RenderMenuList.cpp
Log Message:
-----------
heap-use-after-free | WebCore::RenderMenuList::setTextFromOption;
WebCore::HTMLSelectElement::selectOption; WebCore::Element::didAddAttribute
https://bugs.webkit.org/show_bug.cgi?id=272882
rdar://126279123
Reviewed by Antti Koivisto.
On macOS, `<select>` and `<select multiple>` use `RenderMenuList` and
`RenderMenuList` as their respective renderers. Consequently, whenever the
`multiple` attribute is added, `invalidateStyleAndRenderersForSubtree` is
called and the `RenderMenuList` is marked for destruction.
Additionally, for interoperability, the selected index must be updated when the
`multiple` attribute is added or removed. This update will also trigger an
update on the renderer, in this case, via `RenderMenuList::updateFromElement`.
At this point, the element is `<select multiple>`, but still has a
`RenderMenuList`.
Eventually, the update gets into `RenderMenuList::setTextFromOption`, which
calls `computedStyle()` on an `<option>` element. Following 267786@main, when
using containment, this triggers a render tree update, as
`Document::resolveStyle`
is called, and `resolver.hasUnresolvedQueryContainers()` is true. The
`RenderMenuList` is then destroyed, as it was previously invalidated, while
inside one of its own methods. Use-after-free is then encountered due to
attempted
member variable access.
To fix, take a similar approach as the crash fix in 272334@main and elide a full
style update when a query container with invalid style is encountered.
`fast/css/container-query-listbox.html` has been added to ensure <option>
styling continues to work with container queries. Finally, adopt `CheckedPtr` as
a hardening measure.
Alternatives considered:
1. Call `updateStyleIfNeeded()` in `HTMLSelectElement` prior to entering the
renderer. This approach was rejected as there are too many entry points, and
it would be fragile to new entry points.
2. Pass `<option>` style down from `HTMLSelectElement` into the renderer. Again,
there are too many entry points (including outside of the element).
Additionally,
it is not sufficient to store a single style (for the selected option), as
every
`<option>` participates in width determination.
3. Use `existingComputedStyle()` instead of `computedStyle()`. This resulted in
paint time regressions where the existing computed style was empty.
* LayoutTests/fast/css/container-query-listbox-expected.html: Added.
* LayoutTests/fast/css/container-query-listbox.html: Added.
*
LayoutTests/fast/forms/select-multiple-changed-with-containment-crash-expected.txt:
Added.
* LayoutTests/fast/forms/select-multiple-changed-with-containment-crash.html:
Added.
* LayoutTests/platform/ios/TestExpectations:
* Source/WebCore/html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::optionSelectedByUser):
(WebCore::HTMLSelectElement::selectOption):
* Source/WebCore/rendering/RenderListBox.cpp:
(WebCore::RenderListBox::paintItemForeground):
(WebCore::RenderListBox::paintItemBackground):
* Source/WebCore/rendering/RenderMenuList.cpp:
(RenderMenuList::updateOptionsWidth):
(RenderMenuList::setTextFromOption):
(RenderMenuList::itemStyle const):
(RenderMenuList::getItemBackgroundColor const):
Originally-landed-as: 272448.982@safari-7618-branch (c4b6c7757697).
rdar://132958569
Canonical link: https://commits.webkit.org/281864@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
