Bug 145842 – Network process crashes decoding invalid cache entry on 32bit system

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Antti Koivisto]

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_INVALID_ADDRESS at 0x06000000
Triggered by Thread:  13

Thread 13 name:  Dispatch queue: com.apple.libdispatch-io.opq
Thread 13 Crashed:
0   JavaScriptCore                	0x24db0ef4 WTF::StringImpl::createUninitialized(unsigned int, unsigned char*&) + 36 (StringImpl.h:188)
1   WebKit                        	0x286f1ba8 WebKit::NetworkCache::Coder<WTF::String>::decode(WebKit::NetworkCache::Decoder&, WTF::String&) + 76 (WTFString.h:364)
2   WebKit                        	0x286f4bbc WebKit::NetworkCache::Key::decode(WebKit::NetworkCache::Decoder&, WebKit::NetworkCache::Key&) + 48 (NetworkCacheDecoder.h:76)
3   WebKit                        	0x286f951e std::__1::__function::__func<WebKit::NetworkCache::decodeRecordMetaData(WebKit::NetworkCache::RecordMetaData&, WebKit::NetworkCache::Data const&)::$_13, std::__1::allocator<WebKit::NetworkCache::decodeRecordMetaData(WebKit::NetworkCache::RecordMetaData&, WebKit::NetworkCache::Data const&)::$_13>, bool (unsigned char const*, unsigned long)>::operator()(unsigned char const*&&, unsigned long&&) + 42 (NetworkCacheCoder.h:45)
4   WebKit                        	0x286f1edc ___ZNK6WebKit12NetworkCache4Data5applyEOKNSt3__18functionIFbPKhmEEE_block_invoke + 24 (functional:1793)

[Antti Koivisto]

Created attachment 254653 [details]
patch

[Antti Koivisto]

rdar://problem/21228334

[Antti Koivisto]

Created attachment 254670 [details]
patch

[Said Abou-Hallawa]

Comment on attachment 254670 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254670&action=review

> Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp:52
> +    return currentOffset() + size <= m_bufferSize;

Does not the following code fix the overflow issue regardless whether it runs on 32 or 64bit?

    return size <= m_bufferEnd - m_bufferPosition;

Otherwise I would suggest changing the last statement to be:

   return size <= m_bufferSize - currentOffset();

Subtraction is always safer when dealing with large numbers but they have to have the same sign.

[Antti Koivisto]

Created attachment 254678 [details]
patch 2

[Antti Koivisto]

>     return size <= m_bufferEnd - m_bufferPosition;

Yeah, that's a better idea. Did that instead.

[Antti Koivisto]

http://trac.webkit.org/changeset/185458