Diff
Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/ChangeLog 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog 2021-02-24 17:10:27 UTC (rev 273395)
@@ -1,5 +1,66 @@
2021-02-23 Alan Coon <alanc...@apple.com>
+ Cherry-pick r273196. rdar://problem/74623520
+
+ Crash under Decoder::Decoder()
+ https://bugs.webkit.org/show_bug.cgi?id=222192
+ <rdar://31392681>
+
+ Reviewed by Geoffrey Garen.
+
+ We are sometimes crashing under Decoder's copyBuffer(), inside the memcpy()
+ call, with a null address. I have no idea how this is happening and this
+ code has not changed in a long time so I have made the following hardening:
+ 1. Update copyBuffer() to use tryFastMalloc() instead of fastMalloc(). Log
+ and return null if tryFastMalloc() failed instead of calling memcpy().
+ 2. Update Decoder::create() to log and return early if the input buffer
+ is null.
+ 3. Update Connection's createMessageDecoder() to use CheckedSize when
+ computing the bodySize that is being passed to Decoder::create(). If
+ we overflow, log and return null.
+
+ No new tests, no idea how this can happen in practice.
+
+ * Platform/IPC/Decoder.cpp:
+ (IPC::copyBuffer):
+ (IPC::Decoder::create):
+ (IPC::Decoder::Decoder):
+ * Platform/IPC/cocoa/ConnectionCocoa.mm:
+ (IPC::createMessageDecoder):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273196 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-02-19 Chris Dumez <cdu...@apple.com>
+
+ Crash under Decoder::Decoder()
+ https://bugs.webkit.org/show_bug.cgi?id=222192
+ <rdar://31392681>
+
+ Reviewed by Geoffrey Garen.
+
+ We are sometimes crashing under Decoder's copyBuffer(), inside the memcpy()
+ call, with a null address. I have no idea how this is happening and this
+ code has not changed in a long time so I have made the following hardening:
+ 1. Update copyBuffer() to use tryFastMalloc() instead of fastMalloc(). Log
+ and return null if tryFastMalloc() failed instead of calling memcpy().
+ 2. Update Decoder::create() to log and return early if the input buffer
+ is null.
+ 3. Update Connection's createMessageDecoder() to use CheckedSize when
+ computing the bodySize that is being passed to Decoder::create(). If
+ we overflow, log and return null.
+
+ No new tests, no idea how this can happen in practice.
+
+ * Platform/IPC/Decoder.cpp:
+ (IPC::copyBuffer):
+ (IPC::Decoder::create):
+ (IPC::Decoder::Decoder):
+ * Platform/IPC/cocoa/ConnectionCocoa.mm:
+ (IPC::createMessageDecoder):
+
+2021-02-23 Alan Coon <alanc...@apple.com>
+
Cherry-pick r273180. rdar://problem/74623601
CrashTracer: com.apple.WebKit.Networking at WebKit: WTF::Detail::CallableWrapper<WebKit::PrivateClickMeasurementManager::firePendingAttributionRequests()
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp 2021-02-24 17:10:27 UTC (rev 273395)
@@ -28,6 +28,7 @@
#include "ArgumentCoders.h"
#include "DataReference.h"
+#include "Logging.h"
#include "MessageFlags.h"
#include <stdio.h>
#include <wtf/StdLibExtras.h>
@@ -40,20 +41,39 @@
static const uint8_t* copyBuffer(const uint8_t* buffer, size_t bufferSize)
{
- auto bufferCopy = static_cast<uint8_t*>(fastMalloc(bufferSize));
+ uint8_t* bufferCopy;
+ if (!tryFastMalloc(bufferSize).getValue(bufferCopy)) {
+ RELEASE_LOG_FAULT(IPC, "Decoder::copyBuffer: tryFastMalloc(%lu) failed", bufferSize);
+ return nullptr;
+ }
+
memcpy(bufferCopy, buffer, bufferSize);
-
return bufferCopy;
}
std::unique_ptr<Decoder> Decoder::create(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&& attachments)
{
- auto decoder = makeUnique<Decoder>(buffer, bufferSize, bufferDeallocator, WTFMove(attachments));
+ ASSERT(buffer);
+ if (UNLIKELY(!buffer)) {
+ RELEASE_LOG_FAULT(IPC, "Decoder::create() called with a null buffer (bufferSize: %lu)", bufferSize);
+ return nullptr;
+ }
+
+ const uint8_t* bufferCopy;
+ if (!bufferDeallocator) {
+ bufferCopy = copyBuffer(buffer, bufferSize);
+ ASSERT(bufferCopy);
+ if (UNLIKELY(!bufferCopy))
+ return nullptr;
+ } else
+ bufferCopy = buffer;
+
+ auto decoder = std::unique_ptr<Decoder>(new Decoder(bufferCopy, bufferSize, bufferDeallocator, WTFMove(attachments)));
return decoder->isValid() ? WTFMove(decoder) : nullptr;
}
Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&& attachments)
- : m_buffer { bufferDeallocator ? buffer : copyBuffer(buffer, bufferSize) }
+ : m_buffer { buffer }
, m_bufferPos { m_buffer }
, m_bufferEnd { m_buffer + bufferSize }
, m_bufferDeallocator { bufferDeallocator }
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h 2021-02-24 17:10:27 UTC (rev 273395)
@@ -48,7 +48,6 @@
WTF_MAKE_FAST_ALLOCATED;
public:
static std::unique_ptr<Decoder> create(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&);
- explicit Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&);
~Decoder();
Decoder(const Decoder&) = delete;
@@ -183,6 +182,8 @@
}
private:
+ Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&);
+
enum ConstructWithoutHeaderTag { ConstructWithoutHeader };
Decoder(const uint8_t* buffer, size_t bufferSize, ConstructWithoutHeaderTag);
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm 2021-02-24 17:10:27 UTC (rev 273395)
@@ -28,6 +28,7 @@
#import "DataReference.h"
#import "ImportanceAssertion.h"
+#import "Logging.h"
#import "MachMessage.h"
#import "MachPort.h"
#import "MachUtilities.h"
@@ -419,9 +420,14 @@
if (!(header->msgh_bits & MACH_MSGH_BITS_COMPLEX)) {
// We have a simple message.
uint8_t* body = reinterpret_cast<uint8_t*>(header + 1);
- size_t bodySize = header->msgh_size - sizeof(mach_msg_header_t);
+ auto bodySize = CheckedSize { header->msgh_size } - sizeof(mach_msg_header_t);
+ if (UNLIKELY(bodySize.hasOverflowed())) {
+ RELEASE_LOG_FAULT(IPC, "createMessageDecoder: Overflow when computing bodySize (header->msgh_size: %lu, sizeof(mach_msg_header_t): %lu)", static_cast<unsigned long>(header->msgh_size), sizeof(mach_msg_header_t));
+ ASSERT_NOT_REACHED();
+ return nullptr;
+ }
- return Decoder::create(body, bodySize, nullptr, Vector<Attachment> { });
+ return Decoder::create(body, bodySize.unsafeGet(), nullptr, Vector<Attachment> { });
}
mach_msg_body_t* body = reinterpret_cast<mach_msg_body_t*>(header + 1);
@@ -466,9 +472,15 @@
}
uint8_t* messageBody = descriptorData;
- size_t messageBodySize = header->msgh_size - (descriptorData - reinterpret_cast<uint8_t*>(header));
+ ASSERT(descriptorData >= reinterpret_cast<uint8_t*>(header));
+ auto messageBodySize = CheckedSize { header->msgh_size } - static_cast<size_t>(descriptorData - reinterpret_cast<uint8_t*>(header));
+ if (UNLIKELY(messageBodySize.hasOverflowed())) {
+ RELEASE_LOG_FAULT(IPC, "createMessageDecoder: Overflow when computing bodySize (header->msgh_size: %lu, (descriptorData - reinterpret_cast<uint8_t*>(header)): %lu)", static_cast<unsigned long>(header->msgh_size), static_cast<unsigned long>(descriptorData - reinterpret_cast<uint8_t*>(header)));
+ ASSERT_NOT_REACHED();
+ return nullptr;
+ }
- return Decoder::create(messageBody, messageBodySize, nullptr, WTFMove(attachments));
+ return Decoder::create(messageBody, messageBodySize.unsafeGet(), nullptr, WTFMove(attachments));
}
// The receive buffer size should always include the maximum trailer size.
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp 2021-02-24 17:10:27 UTC (rev 273395)
@@ -223,7 +223,10 @@
if (messageInfo.isBodyOutOfLine())
messageBody = reinterpret_cast<uint8_t*>(oolMessageBody->data());
- auto decoder = makeUnique<Decoder>(messageBody, messageInfo.bodySize(), nullptr, WTFMove(attachments));
+ auto decoder = Decoder::create(messageBody, messageInfo.bodySize(), nullptr, WTFMove(attachments));
+ ASSERT(decoder);
+ if (!decoder)
+ return false;
processIncomingMessage(WTFMove(decoder));
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/win/ConnectionWin.cpp (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/win/ConnectionWin.cpp 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/win/ConnectionWin.cpp 2021-02-24 17:10:27 UTC (rev 273395)
@@ -139,8 +139,10 @@
if (!m_readBuffer.isEmpty()) {
// We have a message, let's dispatch it.
- Vector<Attachment> attachments(0);
- auto decoder = makeUnique<Decoder>(m_readBuffer.data(), m_readBuffer.size(), nullptr, WTFMove(attachments));
+ auto decoder = Decoder::create(m_readBuffer.data(), m_readBuffer.size(), nullptr, { });
+ ASSERT(decoder);
+ if (!decoder)
+ return;
processIncomingMessage(WTFMove(decoder));
}
Modified: branches/safari-611-branch/Source/WebKit/UIProcess/LegacySessionStateCodingNone.cpp (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/UIProcess/LegacySessionStateCodingNone.cpp 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/LegacySessionStateCodingNone.cpp 2021-02-24 17:10:27 UTC (rev 273395)
@@ -48,22 +48,24 @@
bool decodeLegacySessionState(const uint8_t* data, size_t dataSize, SessionState& sessionState)
{
- IPC::Decoder decoder(data, dataSize, nullptr, Vector<IPC::Attachment>());
+ auto decoder = IPC::Decoder::create(data, dataSize, nullptr, Vector<IPC::Attachment>());
+ if (!decoder)
+ return false;
Optional<BackForwardListState> backForwardListState;
- decoder >> backForwardListState;
+ *decoder >> backForwardListState;
if (!backForwardListState)
return false;
sessionState.backForwardListState = WTFMove(*backForwardListState);
Optional<uint64_t> renderTreeSize;
- decoder >> renderTreeSize;
+ *decoder >> renderTreeSize;
if (!renderTreeSize)
return false;
sessionState.renderTreeSize = *renderTreeSize;
Optional<URL> provisionalURL;
- decoder >> provisionalURL;
+ *decoder >> provisionalURL;
if (!provisionalURL)
return false;
sessionState.provisionalURL = WTFMove(*provisionalURL);
Modified: branches/safari-611-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp (273394 => 273395)
--- branches/safari-611-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2021-02-24 17:10:23 UTC (rev 273394)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2021-02-24 17:10:27 UTC (rev 273395)
@@ -396,7 +396,11 @@
if (message.encoder->messageName() == IPC::MessageName::WebPage_LoadRequestWaitingForProcessLaunch) {
auto buffer = message.encoder->buffer();
auto bufferSize = message.encoder->bufferSize();
- std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
+ auto decoder = IPC::Decoder::create(buffer, bufferSize, nullptr, { });
+ ASSERT(decoder);
+ if (!decoder)
+ return false;
+
LoadParameters loadParameters;
URL resourceDirectoryURL;
WebPageProxyIdentifier pageID;
