Skip to content

Conversation

@achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Feb 12, 2024

272b9be

Move VKCImageAnalysis deserialization away from other NSObject deserializers
https://bugs.webkit.org/show_bug.cgi?id=269230
rdar://122825380

Reviewed by Wenson Hsieh.

It is currently the only ObjC object that is deserialized from IPC without
_enableStrictSecureDecodingMode, which is less urgent because it is only sent
from the trusted UI process to the untrusted web content process.  This PR just
moves that logic away from the rest of the IPC logic and adds a
release assertion to make sure we don't introduce a security issue in the future.

* Source/WebCore/platform/TextRecognitionResult.h:
* Source/WebCore/platform/cocoa/TextRecognitionResultCocoa.mm:
(WebCore::TextRecognitionResult::encodeVKCImageAnalysis):
(WebCore::TextRecognitionResult::decodeVKCImageAnalysis):
(WebCore::stringForRange):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::installImageOverlay):
* Source/WebKit/Platform/cocoa/ImageAnalysisUtilities.mm:
(WebKit::makeTextRecognitionResult):
* Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm:
(IPC::shouldEnableStrictMode):
* Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:
(IPC::getClass<VKCImageAnalysis>): Deleted.
(IPC::ArgumentCoder<RetainPtr<VKCImageAnalysis>>::encode): Deleted.
(IPC::ArgumentCoder<RetainPtr<VKCImageAnalysis>>::decode): Deleted.
* Source/WebKit/Shared/TextRecognitionResult.serialization.in:
* Source/WebKit/Shared/WebCoreArgumentCoders.h:

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

19af88b

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios 🛠 mac 🛠 wpe 🛠 wincairo
✅ 🧪 bindings 🛠 ios-sim 🛠 mac-AS-debug 🧪 wpe-wk2
🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 🛠 gtk
🧪 api-ios 🧪 mac-wk2 🧪 gtk-wk2
🛠 tv 🧪 mac-AS-debug-wk2 🧪 api-gtk
🛠 tv-sim 🧪 mac-wk2-stress
🛠 watch
✅ 🛠 🧪 unsafe-merge 🛠 watch-sim

@achristensen07 achristensen07 self-assigned this Feb 12, 2024
@achristensen07 achristensen07 added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Feb 12, 2024
Copy link
Member

@whsieh whsieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@achristensen07 achristensen07 force-pushed the eng/Move-VKCImageAnalysis-deserialization-away-from-other-NSObject-deserializers branch from 0efc39b to be07827 Compare February 12, 2024 21:21
@achristensen07 achristensen07 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Feb 12, 2024
@achristensen07 achristensen07 force-pushed the eng/Move-VKCImageAnalysis-deserialization-away-from-other-NSObject-deserializers branch from be07827 to 19af88b Compare February 12, 2024 22:58
@achristensen07 achristensen07 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Feb 12, 2024
…alizers

https://bugs.webkit.org/show_bug.cgi?id=269230
rdar://122825380

Reviewed by Wenson Hsieh.

It is currently the only ObjC object that is deserialized from IPC without
_enableStrictSecureDecodingMode, which is less urgent because it is only sent
from the trusted UI process to the untrusted web content process.  This PR just
moves that logic away from the rest of the IPC logic and adds a
release assertion to make sure we don't introduce a security issue in the future.

* Source/WebCore/platform/TextRecognitionResult.h:
* Source/WebCore/platform/cocoa/TextRecognitionResultCocoa.mm:
(WebCore::TextRecognitionResult::encodeVKCImageAnalysis):
(WebCore::TextRecognitionResult::decodeVKCImageAnalysis):
(WebCore::stringForRange):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::installImageOverlay):
* Source/WebKit/Platform/cocoa/ImageAnalysisUtilities.mm:
(WebKit::makeTextRecognitionResult):
* Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm:
(IPC::shouldEnableStrictMode):
* Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:
(IPC::getClass<VKCImageAnalysis>): Deleted.
(IPC::ArgumentCoder<RetainPtr<VKCImageAnalysis>>::encode): Deleted.
(IPC::ArgumentCoder<RetainPtr<VKCImageAnalysis>>::decode): Deleted.
* Source/WebKit/Shared/TextRecognitionResult.serialization.in:
* Source/WebKit/Shared/WebCoreArgumentCoders.h:

Canonical link: https://commits.webkit.org/274495@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Move-VKCImageAnalysis-deserialization-away-from-other-NSObject-deserializers branch from 19af88b to 272b9be Compare February 12, 2024 23:06
@webkit-commit-queue webkit-commit-queue merged commit 272b9be into WebKit:main Feb 12, 2024
@webkit-commit-queue
Copy link
Collaborator

Committed 274495@main (272b9be): https://commits.webkit.org/274495@main

Reviewed commits have been landed. Closing PR #24275 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants