KeyedDecoderGeneric fails to allocate Vector while decoding broken data AppleWin WK1 and WinCairo WK1 are sharing same data directory even though they are using different KeyedEncoder/KeyedDecoder format. 1. Start AppleWin WK1 MiniBrowser, Open Web Inspector, Change Setting, for example, zoom scale, Close the AppleWin WK1 2. Start WinCairo WK1 MiniBrowser, Open Web Inspector 3. Crash Callstack: > WTF.dll!WTFCrash() Line 305 C++ > WebKit.dll!WTF::VectorBufferBase<unsigned char,WTF::FastMalloc>::allocateBuffer(unsigned __int64 newCapacity=12884901888) Line 290 C++ > WebKit.dll!WTF::VectorBuffer<unsigned char,0,WTF::FastMalloc>::VectorBuffer<unsigned char,0,WTF::FastMalloc>(unsigned __int64 capacity=12884901888, unsigned __int64 size=12884901888) Line 394 C++ > WebKit.dll!WTF::Vector<unsigned char,0,WTF::CrashOnOverflow,16,WTF::FastMalloc>::Vector<unsigned char,0,WTF::CrashOnOverflow,16,WTF::FastMalloc>(unsigned __int64 size=12884901888) Line 630 C++ > WebKit.dll!WebCore::readString(WTF::Persistence::Decoder & decoder={...}, WTF::String & result={...}) Line 62 C++ > WebKit.dll!WebCore::KeyedDecoderGeneric::KeyedDecoderGeneric(const unsigned char * data=0x0000021665b51690, unsigned __int64 size=53) Line 104 C++ > [External Code] > WebKit.dll!WTF::makeUnique<WebCore::KeyedDecoderGeneric,unsigned char const * &,unsigned __int64 &>(const unsigned char * & <args_0>=0x0000021665b51690, unsigned __int64 & <args_1>=53) Line 483 C++ > WebKit.dll!WebCore::KeyedDecoder::decoder(const unsigned char * data=0x0000021665b51690, unsigned __int64 size=53) Line 88 C++ > WebKit.dll!WebCore::deserializeIDBKeyPath(const unsigned char * data=0x0000021665b51690, unsigned __int64 size=53, WTF::Optional<WTF::Variant<WTF::String,WTF::Vector<WTF::String,0,WTF::CrashOnOverflow,16,WTF::FastMalloc>>> & result={...}) Line 72 C++ > WebKit.dll!WebCore::IDBServer::SQLiteIDBBackingStore::extractExistingDatabaseInfo() Line 767 C++ > WebKit.dll!WebCore::IDBServer::SQLiteIDBBackingStore::getOrEstablishDatabaseInfo(WebCore::IDBDatabaseInfo & info={...}) Line 994 C++ > WebKit.dll!WebCore::IDBServer::UniqueIDBDatabase::performCurrentOpenOperation() Line 176 C++ > WebKit.dll!WebCore::IDBServer::UniqueIDBDatabase::handleCurrentOperation() Line 357 C++ > WebKit.dll!WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations() Line 340 C++ > WebKit.dll!WebCore::IDBServer::UniqueIDBDatabase::openDatabaseConnection(WebCore::IDBServer::IDBConnectionToClient & connection={...}, const WebCore::IDBRequestData & requestData={...}) Line 153 C++ > WebKit.dll!WebCore::IDBServer::IDBServer::openDatabase(const WebCore::IDBRequestData & requestData={...}) Line 152 C++ > WebKit.dll!`InProcessIDBServer::openDatabase'::`2'::<lambda_1>::operator()() Line 145 C++ > WebKit.dll!WTF::Detail::CallableWrapper<`InProcessIDBServer::openDatabase'::`2'::<lambda_1>,void>::call() Line 52 C++ > WebKit.dll!WTF::Function<void __cdecl(void)>::operator()() Line 85 C++ > WebKit.dll!WebCore::StorageThread::threadEntryPoint() Line 79 C++ > WebKit.dll!`WebCore::StorageThread::start'::`17'::<lambda_2>::operator()() Line 66 C++ > WebKit.dll!WTF::Detail::CallableWrapper<`WebCore::StorageThread::start'::`17'::<lambda_2>,void>::call() Line 52 C++ > WTF.dll!WTF::Function<void __cdecl(void)>::operator()() Line 85 C++ > WTF.dll!WTF::Thread::entryPoint(WTF::Thread::NewThreadContext * newThreadContext=0x000002164eea6c70) Line 149 C++ > WTF.dll!WTF::wtfThreadEntryPoint(void * data=0x000002164eea6c70) Line 153 C++ > [External Code] In above case, trying to allocate Vector with size=12884901888.
Created attachment 393626 [details] Patch
Comment on attachment 393626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393626&action=review > Tools/ChangeLog:10 > + (TestWebKitAPI::KeyedCoding.DecodeRandomData): Added a new test decoding random data. I think a better test is decoding pseudo-random data with a fixed seed. We don’t want a test that randomly fails.
Comment on attachment 393626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393626&action=review >> Tools/ChangeLog:10 >> + (TestWebKitAPI::KeyedCoding.DecodeRandomData): Added a new test decoding random data. > > I think a better test is decoding pseudo-random data with a fixed seed. We don’t want a test that randomly fails. Agreed. Will do so.
I found two more crash bugs in KeyedDecoderGeneric by changing the random seed.
Comment on attachment 393626 [details] Patch I see the same mistake in: 1) decodeCFData in CertificateInfo.h 2) AuthenticatorResponseData::decode where it also uses ArrayBuffer::create but should be using ArrayBuffer::tryCreate 3) SerializedScriptValue::decode 4) decodeSharedBuffer and decodeTypesAndData in WebCoreArgumentCoders.cpp We need someone to fix all of those. May not be as easy to write tests for those.
OK, I will try to fix them.
Created attachment 393629 [details] Patch
Comment on attachment 393629 [details] Patch Clearing flags on attachment: 393629 Committed r258486: <https://trac.webkit.org/changeset/258486>
All reviewed patches have been landed. Closing bug.
<rdar://problem/60479609>
(In reply to Darin Adler from comment #5) > > I see the same mistake in: > > 1) decodeCFData in CertificateInfo.h > 2) AuthenticatorResponseData::decode where it also uses ArrayBuffer::create > but should be using ArrayBuffer::tryCreate > 3) SerializedScriptValue::decode > 4) decodeSharedBuffer and decodeTypesAndData in WebCoreArgumentCoders.cpp > > We need someone to fix all of those. May not be as easy to write tests for > those. Filed: Bug 209131 – Don't allocate a buffer with the decoded size without ensuring bufferIsLargeEnoughToContain(size)