[webkit-changes] [WebKit/WebKit] 9cf3ea: IPC deserialization: Clip IntRect rather than fail...

[Abrar Rahman Protyasha]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 9cf3ea6461529a8a721197cc381b0d6e90813686
      https://github.com/WebKit/WebKit/commit/9cf3ea6461529a8a721197cc381b0d6e90813686
  Author: Abrar Rahman Protyasha <a_protyasha at apple.com>
  Date:   2023-10-18 (Wed, 18 Oct 2023)

  Changed paths:
    A LayoutTests/security/clip-invalid-rect-2-expected.txt
    A LayoutTests/security/clip-invalid-rect-2.html
    A LayoutTests/security/clip-invalid-rect-expected.txt
    A LayoutTests/security/clip-invalid-rect.html
    M Source/WebCore/platform/graphics/IntRect.cpp
    M Source/WebCore/platform/graphics/IntRect.h
    M Source/WebKit/Shared/EditorState.cpp
    M Source/WebKit/Shared/EditorState.h
    M Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp
    M Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
    M Source/WebKit/WebProcess/WebPage/WebPage.cpp
    M Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

  Log Message:
  -----------
  IPC deserialization: Clip IntRect rather than fail to decode entirely
https://bugs.webkit.org/show_bug.cgi?id=258222
rdar://109925899
Reviewed by Chris Dumez.

We hardened IntRect decoding by using IntRect::isValid as a validator in
262412 at main, however this has the inadvertent effect of terminating a
sending process if they send a degenerately large IntRect (i.e. one that
overflows at x + width or y + height and thus fails IntRect::isValid).

The messages with degerate rects reposinble for recently reported
deserialization crashes are namely `WebPageProxy::RootViewToScreen`,
`WebPageProxy::EditorStateChanged`,
`RemoteLayerTreeDrawingAreaProxy::CommitLayerTree`, and
`WebPageProxy::MouseDidMoveOverElement`. In this patch, we suggest
clipping the IntRect at the IPC sender call-site, thereby avoiding
overflow issues and not crashing the sending process. For the
`EditorState` case, it makes more sense to sanitize the owned rects in
`WebPage::editorState()`, which is a layer before the IPC sender
call-site.

* LayoutTests/security/clip-invalid-rect-expected.txt: Added.
* LayoutTests/security/clip-invalid-rect.html: Added.
* LayoutTests/security/clip-invalid-rect-2-expected.txt: Added.
* LayoutTests/security/clip-invalid-rect-2.html: Added.
Layout tests that crash trying to decode degenerate IntRect messages.

* Source/WebCore/platform/graphics/IntRect.cpp:
(WebCore::IntRect::toRectWithExtentsClippedToNumericLimits const):
* Source/WebCore/platform/graphics/IntRect.h:
Add a function that returns an IntRect with clipped size such that there
is no int32_t overflow at x + width or y + height.

* Source/WebKit/Shared/EditorState.cpp:
* Source/WebKit/Shared/EditorState.h:
(WebKit::EditorState::clipOwnedRectExtentsToNumericLimits):
Add a function that performs clipping of the rects owned by an EditorState
object.

* Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
Drive-by fix to improve readability of a conditional directive.

* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::mouseDidMoveOverElement):
Perform `IntRect` clipping before sending a `MouseDidMoveOverElement` IPC
message.

* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::editorState const):
Perform `IntRect` clipping for all the rects owned by an EditorState,
thus sanitizing both the `EditorStateChanged` and the `CommitLayerTree`
IPC messages.
(WebKit::WebPage::rootViewToScreen):
Perform `IntRect` clipping before sending a `RootViewToScreen` IPC
message.

* Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::performImmediateActionHitTestAtLocation):
Perform `IntRect` clipping before sending a `MouseDidMoveOverElement` IPC
message.

Canonical link: https://commits.webkit.org/265870.4@safari-7616-branch

Canonical link: https://commits.webkit.org/269494@main