Modified: trunk/Source/WebKit/ChangeLog (285864 => 285865)
--- trunk/Source/WebKit/ChangeLog 2021-11-16 16:47:40 UTC (rev 285864)
+++ trunk/Source/WebKit/ChangeLog 2021-11-16 16:51:30 UTC (rev 285865)
@@ -1,3 +1,23 @@
+2021-11-16 Chris Dumez <cdu...@apple.com>
+
+ Do some hardening in IPC::createMessageDecoder()
+ https://bugs.webkit.org/show_bug.cgi?id=233148
+ <rdar://75139294>
+
+ Reviewed by Darin Adler.
+
+ Do more bound validation insde createMessageDecoder() to make sure we stay within
+ the bounds of our ReceiveBuffer.
+
+ Also, when the body is out of line, set `out_of_line.deallocate` to false since
+ we are taking ownership of the memory and will vm_deallocate() it ourselves.
+ Normally the sender (Connection::sendOutgoingMessage) sets that flag to false but
+ it is better not to rely on the sender setting a particular flag.
+
+ * Platform/IPC/cocoa/ConnectionCocoa.mm:
+ (IPC::createMessageDecoder):
+ (IPC::Connection::receiveSourceEventHandler):
+
2021-11-16 J Pascoe <j_pas...@apple.com>
[WebAuthn] WebKitTestRunner/TWAPI lacks an entitlement and bundle identifier to use required [ASCAgent performAuthorizationRequestsForContext]
Modified: trunk/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm (285864 => 285865)
--- trunk/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm 2021-11-16 16:47:40 UTC (rev 285864)
+++ trunk/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm 2021-11-16 16:51:30 UTC (rev 285865)
@@ -408,8 +408,14 @@
sendOutgoingMessages();
}
-static std::unique_ptr<Decoder> createMessageDecoder(mach_msg_header_t* header)
+static std::unique_ptr<Decoder> createMessageDecoder(mach_msg_header_t* header, size_t bufferSize)
{
+ if (UNLIKELY(header->msgh_size > bufferSize)) {
+ RELEASE_LOG_FAULT(IPC, "createMessageDecoder: msgh_size is greater than bufferSize (header->msgh_size: %lu, bufferSize: %lu)", static_cast<unsigned long>(header->msgh_size), bufferSize);
+ ASSERT_NOT_REACHED();
+ return nullptr;
+ }
+
if (!(header->msgh_bits & MACH_MSGH_BITS_COMPLEX)) {
// We have a simple message.
uint8_t* body = reinterpret_cast<uint8_t*>(header + 1);
@@ -426,9 +432,16 @@
mach_msg_body_t* body = reinterpret_cast<mach_msg_body_t*>(header + 1);
mach_msg_size_t numberOfPortDescriptors = body->msgh_descriptor_count;
ASSERT(numberOfPortDescriptors);
- if (!numberOfPortDescriptors)
+ if (UNLIKELY(!numberOfPortDescriptors))
return nullptr;
+ auto sizeWithPortDescriptors = CheckedSize { sizeof(mach_msg_header_t) + sizeof(mach_msg_body_t) } + CheckedSize { numberOfPortDescriptors } * sizeof(mach_msg_port_descriptor_t);
+ if (UNLIKELY(sizeWithPortDescriptors.hasOverflowed() || sizeWithPortDescriptors.value() > bufferSize)) {
+ RELEASE_LOG_FAULT(IPC, "createMessageDecoder: Overflow when computing sizeWithPortDescriptors (numberOfPortDescriptors: %lu)", static_cast<unsigned long>(numberOfPortDescriptors));
+ ASSERT_NOT_REACHED();
+ return nullptr;
+ }
+
uint8_t* descriptorData = reinterpret_cast<uint8_t*>(body + 1);
// If the message body was sent out-of-line, don't treat the last descriptor
@@ -445,7 +458,7 @@
if (descriptor->type.type != MACH_MSG_PORT_DESCRIPTOR)
return nullptr;
- attachments[numberOfAttachments - i - 1] = Attachment(descriptor->port.name, descriptor->port.disposition);
+ attachments[numberOfAttachments - i - 1] = Attachment { descriptor->port.name, descriptor->port.disposition };
descriptorData += sizeof(mach_msg_port_descriptor_t);
}
@@ -457,6 +470,7 @@
uint8_t* messageBody = static_cast<uint8_t*>(descriptor->out_of_line.address);
size_t messageBodySize = descriptor->out_of_line.size;
+ descriptor->out_of_line.deallocate = false; // We are taking ownership of the memory.
return Decoder::create(messageBody, messageBodySize, [](const uint8_t* buffer, size_t length) {
// FIXME: <rdar://problem/62086358> bufferDeallocator block ignores mach_msg_ool_descriptor_t->deallocate
@@ -465,10 +479,10 @@
}
uint8_t* messageBody = descriptorData;
- ASSERT(descriptorData >= reinterpret_cast<uint8_t*>(header));
- auto messageBodySize = CheckedSize { header->msgh_size } - static_cast<size_t>(descriptorData - reinterpret_cast<uint8_t*>(header));
+ ASSERT((reinterpret_cast<uint8_t*>(header) + sizeWithPortDescriptors.value()) == messageBody);
+ auto messageBodySize = header->msgh_size - sizeWithPortDescriptors;
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)));
+ RELEASE_LOG_FAULT(IPC, "createMessageDecoder: Overflow when computing bodySize (header->msgh_size: %lu, sizeWithPortDescriptors: %lu)", static_cast<unsigned long>(header->msgh_size), static_cast<unsigned long>(sizeWithPortDescriptors.value()));
ASSERT_NOT_REACHED();
return nullptr;
}
@@ -537,7 +551,7 @@
return;
}
- std::unique_ptr<Decoder> decoder = createMessageDecoder(header);
+ std::unique_ptr<Decoder> decoder = createMessageDecoder(header, buffer.size());
if (!decoder)
return;
