Bug 145842 - Network process crashes decoding invalid cache entry on 32bit system
Summary: Network process crashes decoding invalid cache entry on 32bit system
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-10 07:27 PDT by Antti Koivisto
Modified: 2015-06-11 11:23 PDT (History)
4 users (show)

See Also:


Attachments
patch (7.04 KB, patch)
2015-06-10 07:45 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (7.07 KB, patch)
2015-06-10 12:13 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch 2 (6.40 KB, patch)
2015-06-10 14:52 PDT, Antti Koivisto
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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