Changeset 273196 in webkit
- Timestamp:
- Feb 19, 2021 10:56:06 PM (3 years ago)
- Location:
- trunk/Source/WebKit
- Files:
-
- 8 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r273195 r273196 1 2021-02-19 Chris Dumez <cdumez@apple.com> 2 3 Crash under Decoder::Decoder() 4 https://bugs.webkit.org/show_bug.cgi?id=222192 5 <rdar://31392681> 6 7 Reviewed by Geoffrey Garen. 8 9 We are sometimes crashing under Decoder's copyBuffer(), inside the memcpy() 10 call, with a null address. I have no idea how this is happening and this 11 code has not changed in a long time so I have made the following hardening: 12 1. Update copyBuffer() to use tryFastMalloc() instead of fastMalloc(). Log 13 and return null if tryFastMalloc() failed instead of calling memcpy(). 14 2. Update Decoder::create() to log and return early if the input buffer 15 is null. 16 3. Update Connection's createMessageDecoder() to use CheckedSize when 17 computing the bodySize that is being passed to Decoder::create(). If 18 we overflow, log and return null. 19 20 No new tests, no idea how this can happen in practice. 21 22 * Platform/IPC/Decoder.cpp: 23 (IPC::copyBuffer): 24 (IPC::Decoder::create): 25 (IPC::Decoder::Decoder): 26 * Platform/IPC/cocoa/ConnectionCocoa.mm: 27 (IPC::createMessageDecoder): 28 1 29 2021-02-19 BJ Burg <bburg@apple.com> 2 30 -
trunk/Source/WebKit/Platform/IPC/Decoder.cpp
r271916 r273196 29 29 #include "ArgumentCoders.h" 30 30 #include "DataReference.h" 31 #include "Logging.h" 31 32 #include "MessageFlags.h" 32 33 #include <stdio.h> … … 41 42 static const uint8_t* copyBuffer(const uint8_t* buffer, size_t bufferSize) 42 43 { 43 auto bufferCopy = static_cast<uint8_t*>(fastMalloc(bufferSize)); 44 uint8_t* bufferCopy; 45 if (!tryFastMalloc(bufferSize).getValue(bufferCopy)) { 46 RELEASE_LOG_FAULT(IPC, "Decoder::copyBuffer: tryFastMalloc(%lu) failed", bufferSize); 47 return nullptr; 48 } 49 44 50 memcpy(bufferCopy, buffer, bufferSize); 45 46 51 return bufferCopy; 47 52 } … … 49 54 std::unique_ptr<Decoder> Decoder::create(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&& attachments) 50 55 { 51 auto decoder = makeUnique<Decoder>(buffer, bufferSize, bufferDeallocator, WTFMove(attachments)); 56 ASSERT(buffer); 57 if (UNLIKELY(!buffer)) { 58 RELEASE_LOG_FAULT(IPC, "Decoder::create() called with a null buffer (bufferSize: %lu)", bufferSize); 59 return nullptr; 60 } 61 62 const uint8_t* bufferCopy; 63 if (!bufferDeallocator) { 64 bufferCopy = copyBuffer(buffer, bufferSize); 65 ASSERT(bufferCopy); 66 if (UNLIKELY(!bufferCopy)) 67 return nullptr; 68 } else 69 bufferCopy = buffer; 70 71 auto decoder = std::unique_ptr<Decoder>(new Decoder(bufferCopy, bufferSize, bufferDeallocator, WTFMove(attachments))); 52 72 return decoder->isValid() ? WTFMove(decoder) : nullptr; 53 73 } 54 74 55 75 Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&& attachments) 56 : m_buffer { buffer Deallocator ? buffer : copyBuffer(buffer, bufferSize)}76 : m_buffer { buffer } 57 77 , m_bufferPos { m_buffer } 58 78 , m_bufferEnd { m_buffer + bufferSize } -
trunk/Source/WebKit/Platform/IPC/Decoder.h
r272058 r273196 51 51 public: 52 52 static std::unique_ptr<Decoder> create(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&); 53 explicit Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&);54 53 ~Decoder(); 55 54 … … 142 141 143 142 private: 143 Decoder(const uint8_t* buffer, size_t bufferSize, void (*bufferDeallocator)(const uint8_t*, size_t), Vector<Attachment>&&); 144 144 145 enum ConstructWithoutHeaderTag { ConstructWithoutHeader }; 145 146 Decoder(const uint8_t* buffer, size_t bufferSize, ConstructWithoutHeaderTag); -
trunk/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm
r272058 r273196 29 29 #import "DataReference.h" 30 30 #import "ImportanceAssertion.h" 31 #import "Logging.h" 31 32 #import "MachMessage.h" 32 33 #import "MachPort.h" … … 420 421 // We have a simple message. 421 422 uint8_t* body = reinterpret_cast<uint8_t*>(header + 1); 422 size_t bodySize = header->msgh_size - sizeof(mach_msg_header_t); 423 424 return Decoder::create(body, bodySize, nullptr, Vector<Attachment> { }); 423 auto bodySize = CheckedSize { header->msgh_size } - sizeof(mach_msg_header_t); 424 if (UNLIKELY(bodySize.hasOverflowed())) { 425 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)); 426 ASSERT_NOT_REACHED(); 427 return nullptr; 428 } 429 430 return Decoder::create(body, bodySize.unsafeGet(), nullptr, Vector<Attachment> { }); 425 431 } 426 432 … … 467 473 468 474 uint8_t* messageBody = descriptorData; 469 size_t messageBodySize = header->msgh_size - (descriptorData - reinterpret_cast<uint8_t*>(header)); 470 471 return Decoder::create(messageBody, messageBodySize, nullptr, WTFMove(attachments)); 475 ASSERT(descriptorData >= reinterpret_cast<uint8_t*>(header)); 476 auto messageBodySize = CheckedSize { header->msgh_size } - static_cast<size_t>(descriptorData - reinterpret_cast<uint8_t*>(header)); 477 if (UNLIKELY(messageBodySize.hasOverflowed())) { 478 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))); 479 ASSERT_NOT_REACHED(); 480 return nullptr; 481 } 482 483 return Decoder::create(messageBody, messageBodySize.unsafeGet(), nullptr, WTFMove(attachments)); 472 484 } 473 485 -
trunk/Source/WebKit/Platform/IPC/unix/ConnectionUnix.cpp
r271626 r273196 238 238 messageBody = reinterpret_cast<uint8_t*>(oolMessageBody->data()); 239 239 240 auto decoder = makeUnique<Decoder>(messageBody, messageInfo.bodySize(), nullptr, WTFMove(attachments)); 240 auto decoder = Decoder::create(messageBody, messageInfo.bodySize(), nullptr, WTFMove(attachments)); 241 ASSERT(decoder); 242 if (!decoder) 243 return false; 241 244 242 245 processIncomingMessage(WTFMove(decoder)); -
trunk/Source/WebKit/Platform/IPC/win/ConnectionWin.cpp
r248846 r273196 140 140 if (!m_readBuffer.isEmpty()) { 141 141 // We have a message, let's dispatch it. 142 Vector<Attachment> attachments(0); 143 auto decoder = makeUnique<Decoder>(m_readBuffer.data(), m_readBuffer.size(), nullptr, WTFMove(attachments)); 142 auto decoder = Decoder::create(m_readBuffer.data(), m_readBuffer.size(), nullptr, { }); 143 ASSERT(decoder); 144 if (!decoder) 145 return; 144 146 processIncomingMessage(WTFMove(decoder)); 145 147 } -
trunk/Source/WebKit/UIProcess/LegacySessionStateCodingNone.cpp
r261254 r273196 49 49 bool decodeLegacySessionState(const uint8_t* data, size_t dataSize, SessionState& sessionState) 50 50 { 51 IPC::Decoder decoder(data, dataSize, nullptr, Vector<IPC::Attachment>()); 51 auto decoder = IPC::Decoder::create(data, dataSize, nullptr, Vector<IPC::Attachment>()); 52 if (!decoder) 53 return false; 52 54 53 55 Optional<BackForwardListState> backForwardListState; 54 decoder >> backForwardListState;56 *decoder >> backForwardListState; 55 57 if (!backForwardListState) 56 58 return false; … … 58 60 59 61 Optional<uint64_t> renderTreeSize; 60 decoder >> renderTreeSize;62 *decoder >> renderTreeSize; 61 63 if (!renderTreeSize) 62 64 return false; … … 64 66 65 67 Optional<URL> provisionalURL; 66 decoder >> provisionalURL;68 *decoder >> provisionalURL; 67 69 if (!provisionalURL) 68 70 return false; -
trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp
r273183 r273196 397 397 auto buffer = message.encoder->buffer(); 398 398 auto bufferSize = message.encoder->bufferSize(); 399 std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { }); 399 auto decoder = IPC::Decoder::create(buffer, bufferSize, nullptr, { }); 400 ASSERT(decoder); 401 if (!decoder) 402 return false; 403 400 404 LoadParameters loadParameters; 401 405 URL resourceDirectoryURL;
Note: See TracChangeset
for help on using the changeset viewer.