Skip to content
Permalink
Browse files
[CoreIPC] Crash in logDiagnosticMessage code
https://bugs.webkit.org/show_bug.cgi?id=224390

Patch by Julian Gonzalez <julian_a_gonzalez@apple.com> on 2021-04-12
Reviewed by Chris Dumez.

Source/WebKit:

Create new WebPageProxy::logDiagnosticMessage APIs designed to be called on messages that
need sanity checking, and hook that up to IPC handlers (while leaving existing APIs
for all other callers).

Test: ipc/analytics-logger-crash.html

* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::logDiagnosticMessageWithValue):
* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::logDiagnosticMessageFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess):
(WebKit::ProvisionalPageProxy::didReceiveMessage):
* UIProcess/ProvisionalPageProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::logDiagnosticMessageFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithResultFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithValueFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithDomainFromWebProcess):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebCoreSupport/WebDiagnosticLoggingClient.cpp:
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessage):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithResult):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithValue):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithEnhancedPrivacy):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithValueDictionary):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithDomain):

LayoutTests:

Add a test for this crasher.

* ipc/analytics-logger-crash-expected.txt: Added.
* ipc/analytics-logger-crash.html: Added.

Canonical link: https://commits.webkit.org/236426@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275861 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Julian Gonzalez authored and webkit-commit-queue committed Apr 13, 2021
1 parent e13df4b commit cafdeafa951ea5a9c8fcb4737c83b38f20761ab8
Showing 11 changed files with 174 additions and 16 deletions.
@@ -1,3 +1,15 @@
2021-04-12 Julian Gonzalez <julian_a_gonzalez@apple.com>

[CoreIPC] Crash in logDiagnosticMessage code
https://bugs.webkit.org/show_bug.cgi?id=224390

Reviewed by Chris Dumez.

Add a test for this crasher.

* ipc/analytics-logger-crash-expected.txt: Added.
* ipc/analytics-logger-crash.html: Added.

2021-04-12 Commit Queue <commit-queue@webkit.org>

Unreviewed, reverting r275793.
@@ -0,0 +1 @@
This test passes if it does not crash.
@@ -0,0 +1,20 @@
<!DOCTYPE html><!-- webkit-test-runner [ IPCTestingAPIEnabled=true ] -->
<html>
<head>
<script>
if (window.testRunner)
testRunner.dumpAsText();

if (window.IPC) {
buf = new Uint8Array([0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x0,
0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0xfb,0x20,0x0,0x0,0x0,
0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,0xda,0xe,0xef,0xd,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x0,0x21,
0x0,0x0,0x0,0x0,0x0,0x21,0x1,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,0x0,0x0,0x21,0x0,0x0,0x0,]);
IPC.sendMessage('UI',IPC.webPageProxyID,IPC.messages.WebPageProxy_LogDiagnosticMessageWithValueFromWebProcess.name,[buf]);
}
</script>
<body>
This test passes if it does not crash.
</body>
</head>
</html>
@@ -1,3 +1,41 @@
2021-04-12 Julian Gonzalez <julian_a_gonzalez@apple.com>

[CoreIPC] Crash in logDiagnosticMessage code
https://bugs.webkit.org/show_bug.cgi?id=224390

Reviewed by Chris Dumez.

Create new WebPageProxy::logDiagnosticMessage APIs designed to be called on messages that
need sanity checking, and hook that up to IPC handlers (while leaving existing APIs
for all other callers).

Test: ipc/analytics-logger-crash.html

* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::logDiagnosticMessageWithValue):
* UIProcess/ProvisionalPageProxy.cpp:
(WebKit::ProvisionalPageProxy::logDiagnosticMessageFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess):
(WebKit::ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess):
(WebKit::ProvisionalPageProxy::didReceiveMessage):
* UIProcess/ProvisionalPageProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::logDiagnosticMessageFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithResultFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithValueFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess):
(WebKit::WebPageProxy::logDiagnosticMessageWithDomainFromWebProcess):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebCoreSupport/WebDiagnosticLoggingClient.cpp:
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessage):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithResult):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithValue):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithEnhancedPrivacy):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithValueDictionary):
(WebKit::WebDiagnosticLoggingClient::logDiagnosticMessageWithDomain):

2021-04-12 Jiewen Tan <jiewen_tan@apple.com>

Force the WebAuthn compatible mode to always show UI
@@ -221,7 +221,7 @@ void NetworkSession::notifyResourceLoadStatisticsProcessed()

void NetworkSession::logDiagnosticMessageWithValue(const String& message, const String& description, unsigned value, unsigned significantFigures, WebCore::ShouldSample shouldSample)
{
m_networkProcess->parentProcessConnection()->send(Messages::WebPageProxy::LogDiagnosticMessageWithValue(message, description, value, significantFigures, shouldSample), 0);
m_networkProcess->parentProcessConnection()->send(Messages::WebPageProxy::LogDiagnosticMessageWithValueFromWebProcess(message, description, value, significantFigures, shouldSample), 0);
}

void NetworkSession::deleteAndRestrictWebsiteDataForRegistrableDomains(OptionSet<WebsiteDataType> dataTypes, RegistrableDomainsToDeleteOrRestrictWebsiteDataFor&& domains, bool shouldNotifyPage, CompletionHandler<void(const HashSet<RegistrableDomain>&)>&& completionHandler)
@@ -49,6 +49,8 @@
#include "WebProcessProxy.h"
#include <WebCore/ShouldTreatAsContinuingLoad.h>

#define MESSAGE_CHECK(process, assertion) MESSAGE_CHECK_BASE(assertion, process->connection())

namespace WebKit {

using namespace WebCore;
@@ -374,6 +376,27 @@ void ProvisionalPageProxy::decidePolicyForNavigationActionSync(FrameIdentifier f
m_page.decidePolicyForNavigationActionSyncShared(m_process.copyRef(), frameID, isMainFrame, WTFMove(frameInfoData), identifier, navigationID, WTFMove(navigationActionData), WTFMove(originatingFrameInfo), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
}

void ProvisionalPageProxy::logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

m_page.logDiagnosticMessage(message, description, shouldSample);
}

void ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

m_page.logDiagnosticMessageWithEnhancedPrivacy(message, description, shouldSample);
}

void ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary& valueDictionary, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

m_page.logDiagnosticMessageWithValueDictionary(message, description, valueDictionary, shouldSample);
}

#if USE(QUICK_LOOK)
void ProvisionalPageProxy::requestPasswordForQuickLookDocumentInMainFrame(const String& fileName, CompletionHandler<void(const String&)>&& completionHandler)
{
@@ -425,9 +448,6 @@ void ProvisionalPageProxy::didReceiveMessage(IPC::Connection& connection, IPC::D
|| decoder.messageName() == Messages::WebPageProxy::DidDestroyNavigation::name()
|| decoder.messageName() == Messages::WebPageProxy::DidFinishProgress::name()
|| decoder.messageName() == Messages::WebPageProxy::BackForwardAddItem::name()
|| decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessage::name()
|| decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessageWithEnhancedPrivacy::name()
|| decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessageWithValueDictionary::name()
|| decoder.messageName() == Messages::WebPageProxy::SetNetworkRequestsInProgress::name()
|| decoder.messageName() == Messages::WebPageProxy::WillGoToBackForwardListItem::name()
#if USE(QUICK_LOOK)
@@ -450,6 +470,21 @@ void ProvisionalPageProxy::didReceiveMessage(IPC::Connection& connection, IPC::D
}
#endif

if (decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessageFromWebProcess::name()) {
IPC::handleMessage<Messages::WebPageProxy::LogDiagnosticMessageFromWebProcess>(decoder, this, &ProvisionalPageProxy::logDiagnosticMessageFromWebProcess);
return;
}

if (decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessageWithEnhancedPrivacyFromWebProcess::name()) {
IPC::handleMessage<Messages::WebPageProxy::LogDiagnosticMessageWithEnhancedPrivacyFromWebProcess>(decoder, this, &ProvisionalPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess);
return;
}

if (decoder.messageName() == Messages::WebPageProxy::LogDiagnosticMessageWithValueDictionaryFromWebProcess::name()) {
IPC::handleMessage<Messages::WebPageProxy::LogDiagnosticMessageWithValueDictionaryFromWebProcess>(decoder, this, &ProvisionalPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess);
return;
}

if (decoder.messageName() == Messages::WebPageProxy::StartURLSchemeTask::name()) {
IPC::handleMessage<Messages::WebPageProxy::StartURLSchemeTask>(decoder, this, &ProvisionalPageProxy::startURLSchemeTask);
return;
@@ -131,6 +131,9 @@ class ProvisionalPageProxy : public IPC::MessageReceiver, public IPC::MessageSen
void didStartProvisionalLoadForFrame(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, URL&&, URL&& unreachableURL, const UserData&);
void didCommitLoadForFrame(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, const String& mimeType, bool frameHasCustomContentProvider, WebCore::FrameLoadType, const WebCore::CertificateInfo&, bool usedLegacyTLS, bool containsPluginDocument, Optional<WebCore::HasInsecureContent> forcedHasInsecureContent, WebCore::MouseEventPolicy, const UserData&);
void didFailProvisionalLoadForFrame(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, const String& provisionalURL, const WebCore::ResourceError&, WebCore::WillContinueLoading, const UserData&);
void logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample);
void logDiagnosticMessageWithEnhancedPrivacyFromWebProcess(const String& message, const String& description, WebCore::ShouldSample);
void logDiagnosticMessageWithValueDictionaryFromWebProcess(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary&, WebCore::ShouldSample);
void startURLSchemeTask(URLSchemeTaskParameters&&);
void backForwardGoToItem(const WebCore::BackForwardItemIdentifier&, CompletionHandler<void(const WebBackForwardListCounts&)>&&);
void decidePolicyForNavigationActionSync(WebCore::FrameIdentifier, bool isMainFrame, FrameInfoData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&, FrameInfoData&& originatingFrameInfo,
@@ -7215,6 +7215,13 @@ void WebPageProxy::logDiagnosticMessage(const String& message, const String& des
effectiveClient->logDiagnosticMessage(this, message, description);
}

void WebPageProxy::logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessage(message, description, shouldSample);
}

void WebPageProxy::logDiagnosticMessageWithResult(const String& message, const String& description, uint32_t result, WebCore::ShouldSample shouldSample)
{
auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
@@ -7224,6 +7231,13 @@ void WebPageProxy::logDiagnosticMessageWithResult(const String& message, const S
effectiveClient->logDiagnosticMessageWithResult(this, message, description, static_cast<WebCore::DiagnosticLoggingResultType>(result));
}

void WebPageProxy::logDiagnosticMessageWithResultFromWebProcess(const String& message, const String& description, uint32_t result, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessageWithResult(message, description, result, shouldSample);
}

void WebPageProxy::logDiagnosticMessageWithValue(const String& message, const String& description, double value, unsigned significantFigures, ShouldSample shouldSample)
{
auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
@@ -7233,6 +7247,13 @@ void WebPageProxy::logDiagnosticMessageWithValue(const String& message, const St
effectiveClient->logDiagnosticMessageWithValue(this, message, description, String::numberToStringFixedPrecision(value, significantFigures));
}

void WebPageProxy::logDiagnosticMessageWithValueFromWebProcess(const String& message, const String& description, double value, unsigned significantFigures, ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessageWithValue(message, description, value, significantFigures, shouldSample);
}

void WebPageProxy::logDiagnosticMessageWithEnhancedPrivacy(const String& message, const String& description, ShouldSample shouldSample)
{
auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
@@ -7242,6 +7263,13 @@ void WebPageProxy::logDiagnosticMessageWithEnhancedPrivacy(const String& message
effectiveClient->logDiagnosticMessageWithEnhancedPrivacy(this, message, description);
}

void WebPageProxy::logDiagnosticMessageWithEnhancedPrivacyFromWebProcess(const String& message, const String& description, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessageWithEnhancedPrivacy(message, description, shouldSample);
}

void WebPageProxy::logDiagnosticMessageWithValueDictionary(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary& valueDictionary, WebCore::ShouldSample shouldSample)
{
auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
@@ -7263,6 +7291,13 @@ void WebPageProxy::logDiagnosticMessageWithValueDictionary(const String& message
effectiveClient->logDiagnosticMessageWithValueDictionary(this, message, description, WTFMove(apiDictionary));
}

void WebPageProxy::logDiagnosticMessageWithValueDictionaryFromWebProcess(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary& valueDictionary, WebCore::ShouldSample shouldSample)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessageWithValueDictionary(message, description, valueDictionary, shouldSample);
}

void WebPageProxy::logDiagnosticMessageWithDomain(const String& message, WebCore::DiagnosticLoggingDomain domain)
{
auto* effectiveClient = effectiveDiagnosticLoggingClient(ShouldSample::No);
@@ -7272,6 +7307,13 @@ void WebPageProxy::logDiagnosticMessageWithDomain(const String& message, WebCore
effectiveClient->logDiagnosticMessageWithDomain(this, message, domain);
}

void WebPageProxy::logDiagnosticMessageWithDomainFromWebProcess(const String& message, WebCore::DiagnosticLoggingDomain domain)
{
MESSAGE_CHECK(m_process, message.isAllASCII());

logDiagnosticMessageWithDomain(message, domain);
}

void WebPageProxy::logScrollingEvent(uint32_t eventType, MonotonicTime timestamp, uint64_t data)
{
PerformanceLoggingClient::ScrollingEvent event = static_cast<PerformanceLoggingClient::ScrollingEvent>(eventType);
@@ -1533,6 +1533,13 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>
void logDiagnosticMessageWithValueDictionary(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary&, WebCore::ShouldSample);
void logDiagnosticMessageWithDomain(const String& message, WebCore::DiagnosticLoggingDomain);

void logDiagnosticMessageFromWebProcess(const String& message, const String& description, WebCore::ShouldSample);
void logDiagnosticMessageWithResultFromWebProcess(const String& message, const String& description, uint32_t result, WebCore::ShouldSample);
void logDiagnosticMessageWithValueFromWebProcess(const String& message, const String& description, double value, unsigned significantFigures, WebCore::ShouldSample);
void logDiagnosticMessageWithEnhancedPrivacyFromWebProcess(const String& message, const String& description, WebCore::ShouldSample);
void logDiagnosticMessageWithValueDictionaryFromWebProcess(const String& message, const String& description, const WebCore::DiagnosticLoggingClient::ValueDictionary&, WebCore::ShouldSample);
void logDiagnosticMessageWithDomainFromWebProcess(const String& message, WebCore::DiagnosticLoggingDomain);

// Performance logging.
void logScrollingEvent(uint32_t eventType, MonotonicTime, uint64_t);

@@ -206,12 +206,12 @@ messages -> WebPageProxy {
ExecuteUndoRedo(enum:bool WebKit::UndoOrRedo undoOrRedo) -> () Synchronous

# Diagnostic messages logging
LogDiagnosticMessage(String message, String description, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithResult(String message, String description, uint32_t result, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithValue(String message, String description, double value, unsigned significantFigures, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithEnhancedPrivacy(String message, String description, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithValueDictionary(String message, String description, WebCore::DiagnosticLoggingClient::ValueDictionary value, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithDomain(String message, WebCore::DiagnosticLoggingDomain domain)
LogDiagnosticMessageFromWebProcess(String message, String description, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithResultFromWebProcess(String message, String description, uint32_t result, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithValueFromWebProcess(String message, String description, double value, unsigned significantFigures, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithEnhancedPrivacyFromWebProcess(String message, String description, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithValueDictionaryFromWebProcess(String message, String description, WebCore::DiagnosticLoggingClient::ValueDictionary value, enum:bool WebCore::ShouldSample shouldSample)
LogDiagnosticMessageWithDomainFromWebProcess(String message, WebCore::DiagnosticLoggingDomain domain)

# Performance logging
LogScrollingEvent(uint32_t eventType, MonotonicTime timestamp, uint64_t data)

0 comments on commit cafdeaf

Please sign in to comment.