[webkit-changes] [WebKit/WebKit] 403141: ASSERTION FAILED: isPublicSuffixCF(publicSuffix) i...

[Sihui]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 4031410eeeb157432f0a6e0ca143e54d02665096
      https://github.com/WebKit/WebKit/commit/4031410eeeb157432f0a6e0ca143e54d02665096
  Author: Sihui Liu <sihui_liu at apple.com>
  Date:   2024-05-22 (Wed, 22 May 2024)

  Changed paths:
    M Source/WebCore/Headers.cmake
    M Source/WebCore/WebCore.xcodeproj/project.pbxproj
    A Source/WebCore/platform/PublicSuffix.h
    M Source/WebCore/platform/PublicSuffixStore.cpp
    M Source/WebCore/platform/PublicSuffixStore.h
    M Source/WebCore/platform/cocoa/PublicSuffixStoreCocoa.mm
    M Source/WebKit/Shared/GoToBackForwardItemParameters.h
    M Source/WebKit/Shared/GoToBackForwardItemParameters.serialization.in
    M Source/WebKit/Shared/LoadParameters.h
    M Source/WebKit/Shared/LoadParameters.serialization.in
    M Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
    M Tools/TestWebKitAPI/Tests/WebCore/PublicSuffix.cpp

  Log Message:
  -----------
  ASSERTION FAILED: isPublicSuffixCF(publicSuffix) in PublicSuffixStore::addPublicSuffix
https://bugs.webkit.org/show_bug.cgi?id=274418
rdar://128255984

Reviewed by Chris Dumez.

The assertion was introduced in 276834 at main to help debug rdar://125417343, where we suspected the cause is unexpected
entry being added to public suffix cache, like 276635 at main. However, because UI process and web process have different
sandbox access, isPublicSuffixCF (_CFHostIsDomainTopLevel) might return different values on the same string. That means,
in web process, the assertion would be hit even though the string is a valid public suffix according to UI process's
check.

As the original goal is to ensure UI process only sends valid public suffix (or empty string) to web process, the patch
does the hardening by introducing a new class PublicSuffix. UI process needs to explicity contruct it from URL with
PublicSuffixStore and send PublicSuffix object to web process, which is less error-prone than allowing UI process sends
public suffix in string. With this change, we no longer need the assertion.

* Source/WebCore/Headers.cmake:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/PublicSuffix.h: Added.
(WebCore::PublicSuffix::fromRawString):
(WebCore::PublicSuffix::isValid const):
(WebCore::PublicSuffix::string const):
(WebCore::PublicSuffix::isolatedCopy const):
(WebCore::PublicSuffix::PublicSuffix):
(WebCore::PublicSuffix::operator== const):
(WebCore::PublicSuffix::isHashTableDeletedValue const):
(WebCore::PublicSuffix::hash const):
(WebCore::PublicSuffix::PublicSuffixHash::hash):
(WebCore::PublicSuffix::PublicSuffixHash::equal):
* Source/WebCore/platform/PublicSuffixStore.cpp:
(WebCore::PublicSuffixStore::publicSuffix const):
* Source/WebCore/platform/PublicSuffixStore.h:
(): Deleted.
* Source/WebCore/platform/cocoa/PublicSuffixStoreCocoa.mm:
(WebCore::PublicSuffixStore::platformIsPublicSuffix const):
(WebCore::PublicSuffixStore::enablePublicSuffixCache):
(WebCore::PublicSuffixStore::addPublicSuffix):
* Source/WebKit/Shared/GoToBackForwardItemParameters.h:
* Source/WebKit/Shared/GoToBackForwardItemParameters.serialization.in:
* Source/WebKit/Shared/LoadParameters.h:
* Source/WebKit/Shared/LoadParameters.serialization.in:
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Tools/TestWebKitAPI/Tests/WebCore/PublicSuffix.cpp:
(TestWebKitAPI::TEST_F):

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



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