Bug 145842

Summary: Network process crashes decoding invalid cache entry on 32bit system
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, cdumez, cgarcia, commit-queue
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch
none
patch 2 andersca: review+

Description Antti Koivisto 2015-06-10 07:27:57 PDT 
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)
Comment 1 Antti Koivisto 2015-06-10 07:45:20 PDT 
Created attachment 254653 [details]
patch
Comment 2 Antti Koivisto 2015-06-10 12:11:39 PDT 
rdar://problem/21228334
Comment 3 Antti Koivisto 2015-06-10 12:13:43 PDT 
Created attachment 254670 [details]
patch
Comment 4 Said Abou-Hallawa 2015-06-10 14:15:37 PDT 
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.
Comment 5 Antti Koivisto 2015-06-10 14:52:50 PDT 
Created attachment 254678 [details]
patch 2
Comment 6 Antti Koivisto 2015-06-10 14:53:48 PDT 
>     return size <= m_bufferEnd - m_bufferPosition;

Yeah, that's a better idea. Did that instead.
Comment 7 Antti Koivisto 2015-06-11 11:23:59 PDT 
http://trac.webkit.org/changeset/185458