[webkit-changes] [WebKit/WebKit] 45a302: RemotePlayback::cancelWatchAvailability should che...

Tue Sep 17 09:39:47 PDT 2024

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 45a302cb1f4e66adf4f258ca6eaf6474f7005ddb
      https://github.com/WebKit/WebKit/commit/45a302cb1f4e66adf4f258ca6eaf6474f7005ddb
  Author: Jer Noble <jer.noble at apple.com>
  Date:   2024-09-17 (Tue, 17 Sep 2024)

  Changed paths:
    M Source/WebCore/Modules/remoteplayback/RemotePlayback.cpp

  Log Message:
  -----------
  RemotePlayback::cancelWatchAvailability should check if id is a valid key
https://bugs.webkit.org/show_bug.cgi?id=276675
rdar://131818994

Reviewed by Brent Fulgham.

* Source/WebCore/Modules/remoteplayback/RemotePlayback.cpp:
(WebCore::RemotePlayback::cancelWatchAvailability):

Originally-landed-as: 280938.10 at safari-7619-branch (ae96f3eb69a4). rdar://136111693
Canonical link: https://commits.webkit.org/283776@main

  Commit: 5b116606568ba6b5d21b5c705f4b18cbf3d4a387
      https://github.com/WebKit/WebKit/commit/5b116606568ba6b5d21b5c705f4b18cbf3d4a387
  Author: Jer Noble <jer.noble at apple.com>
  Date:   2024-09-17 (Tue, 17 Sep 2024)

  Changed paths:
    M Source/WebCore/Modules/webaudio/BaseAudioContext.cpp

  Log Message:
  -----------
  Race condition in BaseAudioContext::decodeAudioData leading to a use-after-free
https://bugs.webkit.org/show_bug.cgi?id=276728
rdar://131528607

Reviewed by Andy Estes.

Pin the ArrayBuffer passed into decodeAudioData(), so that a call to transfer()
will result in the buffer being copied, not moved.

* Source/WebCore/Modules/webaudio/BaseAudioContext.cpp:
(WebCore::BaseAudioContext::decodeAudioData):

Originally-landed-as: 280938.25 at safari-7619-branch (7397c4cb81b3). rdar://136111312
Canonical link: https://commits.webkit.org/283777@main

  Commit: 74125d4190199d6f8cc326bf9b28b4e6f222de72
      https://github.com/WebKit/WebKit/commit/74125d4190199d6f8cc326bf9b28b4e6f222de72
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2024-09-17 (Tue, 17 Sep 2024)

  Changed paths:
    A LayoutTests/fast/events/no-crash-when-removing-focused-subframe-expected.txt
    A LayoutTests/fast/events/no-crash-when-removing-focused-subframe.html
    M Source/WebCore/html/HTMLFrameElementBase.cpp

  Log Message:
  -----------
  Security assertion in `Document::dispatchWindowEvent` while disconnecting a focused iframe
rdar://129233812

Reviewed by Abrar Rahman Protyasha and Ryosuke Niwa.

This fixes a `ScriptDisallowedScope` assertion that's hit in the following chain of events:

(1) Suppose the document contains two subframes: `A` and `B`.

(2) We set `innerHTML` (or `outerHTML`) on the body, replacing the entire document's content. This
    begins the process of removing `A` and `B` from the DOM.

(3) In doing so, `WebCore::disconnectSubframes` iterates over `A` and `B`, detaching them one at a
    time (let's say it detaches `A` first, followed by `B`).

(4) While disconnecting `B` in `disconnectSubframes`, we dispatch a `load` event on the page, due to
    the fact that disconnecting a subframe stopping the subframe load, causing us to tell
    `FrameLoader` to `checkCompleted()`.

(5) In the `load` event, we tell `A` to begin loading again by setting `src` or `srcdoc` to
    anything; this reinitializes the content window and content document. At this point, we now have
    a frame `A` that's attempting to load content, even though it's in the middle of being
    disconnected.

(6) Still in the `load` event handler, we tell `A` to `focus()` — it now becomes the focused
    element, and also contains the focused frame. Note that it has a content frame at this point
    because we were able to re-trigger a load in step (5).

(7) Exiting the `load` event handler, we now continue on with the rest of `set{Inner|Outer}HTML`,
    which now tries to `adjustFocusedNodeOnNodeRemoval`.

(8) Because we made `A`'s frame the focused frame in step (6) and it has a non-null content window
    from step (5), we try to dispatch a blur event on the content window, when calling
    `FocusController::setFocusedFrame(nullptr)`.

Because (8) happens underneath the script not allowed assertion inside of
`ContainerNode::replaceChild` (or really, any one of the similar "script not allowed" assertions),
we crash.

There are multiple approaches we could use to fix this, including:

a.  Deferring `load` until after frames are done disconnecting.

b.  Making it so that `adjustFocusedNodeOnNodeRemoval` avoids dispatching `blur` on a focused
    frame's content window.

c.  Don't allow a subframe to load if it is in a DOM subtree that is being disconnected.

I'm taking approach (c) above in this patch, since it is (i) sufficient to resolve this bug, (ii)
necessary in order to avoid other debug assertions that arise in the test case, and (iii) likely the
least risky change to address this in the short term.

* LayoutTests/fast/events/no-crash-when-removing-focused-subframe-expected.txt: Added.
* LayoutTests/fast/events/no-crash-when-removing-focused-subframe.html: Added.
* Source/WebCore/html/HTMLFrameElementBase.cpp:
(WebCore::HTMLFrameElementBase::setLocation):

Originally-landed-as: 280938.62 at safari-7619-branch (0748b3747d97). rdar://136111174
Canonical link: https://commits.webkit.org/283778@main

  Commit: 0bae25297c7c5b6927a98bbb71ad9fc776d96431
      https://github.com/WebKit/WebKit/commit/0bae25297c7c5b6927a98bbb71ad9fc776d96431
  Author: Nitin Mahendru <nitinmahendru at apple.com>
  Date:   2024-09-17 (Tue, 17 Sep 2024)

  Changed paths:
    M Source/WTF/wtf/text/StringView.h

  Log Message:
  -----------
  Prevent size overflow in StringView::append
https://bugs.webkit.org/show_bug.cgi?id=276829
rdar://132068335

Reviewed by Darin Adler.

using size_t instead of unsigned will prevent integer addition overflow.

Manually tested.

* Source/WTF/wtf/text/StringView.h:
(WTF::append):

Originally-landed-as: 280938.89 at safari-7619-branch (8a58c365d508). rdar://136110889
Canonical link: https://commits.webkit.org/283779@main

  Commit: 16d8e9346f0fde54401ec67b90a9b0b327be83a1
      https://github.com/WebKit/WebKit/commit/16d8e9346f0fde54401ec67b90a9b0b327be83a1
  Author: Nicole Rosario <nicole_rosario at apple.com>
  Date:   2024-09-17 (Tue, 17 Sep 2024)

  Changed paths:
    A LayoutTests/fast/html/decrement-subframes-expected.txt
    A LayoutTests/fast/html/decrement-subframes.html
    M Source/WebCore/html/HTMLFrameOwnerElement.cpp

  Log Message:
  -----------
  Hitting RELEASE_ASSERT(amount <= bitfields.connectedSubframeCount) in decrement subframe
https://bugs.webkit.org/show_bug.cgi?id=276637
rdar://130091196

Reviewed by Ryosuke Niwa.

Over decrements connectedSubframeCount (tries to decrease when it's 0)
due to calling `frame->disconnectOwnerElement()` even if it was already
processed

* Source/WebCore/html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
* LayoutTests/fast/html/decrement-subframes-expected.txt
* LayoutTests/fast/html/decrement-subframes.html

Originally-landed-as: 280938.90 at safari-7619-branch (eb3508a3bd08). rdar://136110756
Canonical link: https://commits.webkit.org/283780@main

Compare: https://github.com/WebKit/WebKit/compare/b5c6650e0ec3...16d8e9346f0f

To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications