<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"

"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">

<html xmlns="http://www.w3.org/1999/xhtml">

<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />

<title>[185458] trunk/Source/WebKit2</title>

</head>

<body>

<style type="text/css"><!--

#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }

#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }

#msg dt:after { content:':';}

#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }

#msg dl a { font-weight: bold}

#msg dl a:link    { color:#fc3; }

#msg dl a:active  { color:#ff0; }

#msg dl a:visited { color:#cc6; }

h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }

#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }

#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }

#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }

#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }

#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }

#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }

#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }

#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }

#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }

#logmsg pre { background: #eee; padding: 1em; }

#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}

#logmsg dl { margin: 0; }

#logmsg dt { font-weight: bold; }

#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }

#logmsg dd:before { content:'\00bb';}

#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }

#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }

#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }

#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }

#logmsg table th.Corner { text-align: left; }

#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }

#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }

#patch { width: 100%; }

#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}

#patch .propset h4, #patch .binary h4 {margin:0;}

#patch pre {padding:0;line-height:1.2em;margin:0;}

#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}

#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}

#patch span {display:block;padding:0 10px;}

#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}

#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}

#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}

#patch .lines, .info {color:#888;background:#fff;}

--></style>

<div id="msg">

<dl class="meta">

<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/185458">185458</a></dd>

<dt>Author</dt> <dd>antti@apple.com</dd>

<dt>Date</dt> <dd>2015-06-11 10:03:02 -0700 (Thu, 11 Jun 2015)</dd>

</dl>

<h3>Log Message</h3>

<pre>Network process crashes decoding invalid cache entry on 32bit system

https://bugs.webkit.org/show_bug.cgi?id=145842

rdar://problem/21228334

Reviewed by Anders Carlsson.

After cache scheme changes we may end up decoding invalid cache entries. This is by design,

we should just fail decoding and delete these entries.

However Decoder::bufferIsLargeEnoughToContain test in some cases would allow corrupted large

sizes due to overflow in 32bit pointer math and we would crash when allocating a string.

* NetworkProcess/cache/NetworkCacheCoders.cpp:

(WebKit::NetworkCache::Coder&lt;CString&gt;::decode):

(WebKit::NetworkCache::decodeStringText):

(WebKit::NetworkCache::Coder&lt;WebCore::CertificateInfo&gt;::decode):

(WebKit::NetworkCache::Coder&lt;MD5::Digest&gt;::encode):

* NetworkProcess/cache/NetworkCacheCoders.h:

* NetworkProcess/cache/NetworkCacheDecoder.cpp:

(WebKit::NetworkCache::Decoder::Decoder):

(WebKit::NetworkCache::Decoder::bufferIsLargeEnoughToContain):

    Reshuffle to avoid sum.

(WebKit::NetworkCache::Decoder::decodeFixedLengthData):

* NetworkProcess/cache/NetworkCacheDecoder.h:

(WebKit::NetworkCache::Decoder::bufferSize):

(WebKit::NetworkCache::Decoder::currentOffset):

(WebKit::NetworkCache::Decoder::length): Deleted.

(WebKit::NetworkCache::Decoder::isInvalid): Deleted.

(WebKit::NetworkCache::Decoder::markInvalid): Deleted.

    Remove these, they are not really used or needed.</pre>

<h3>Modified Paths</h3>

<ul>

<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>

<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheCoderscpp">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.cpp</a></li>

<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheCodersh">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.h</a></li>

<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheDecodercpp">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp</a></li>

<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheDecoderh">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h</a></li>

</ul>

</div>

<div id="patch">

<h3>Diff</h3>

<a id="trunkSourceWebKit2ChangeLog"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (185457 => 185458)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/ChangeLog        2015-06-11 17:01:46 UTC (rev 185457)

+++ trunk/Source/WebKit2/ChangeLog        2015-06-11 17:03:02 UTC (rev 185458)

</span><span class="lines">@@ -1,3 +1,39 @@

</span><ins>+2015-06-11  Antti Koivisto  &lt;antti@apple.com&gt;

+

+        Network process crashes decoding invalid cache entry on 32bit system

+        https://bugs.webkit.org/show_bug.cgi?id=145842

+        rdar://problem/21228334

+

+        Reviewed by Anders Carlsson.

+

+        After cache scheme changes we may end up decoding invalid cache entries. This is by design,

+        we should just fail decoding and delete these entries.

+

+        However Decoder::bufferIsLargeEnoughToContain test in some cases would allow corrupted large

+        sizes due to overflow in 32bit pointer math and we would crash when allocating a string.

+

+        * NetworkProcess/cache/NetworkCacheCoders.cpp:

+        (WebKit::NetworkCache::Coder&lt;CString&gt;::decode):

+        (WebKit::NetworkCache::decodeStringText):

+        (WebKit::NetworkCache::Coder&lt;WebCore::CertificateInfo&gt;::decode):

+        (WebKit::NetworkCache::Coder&lt;MD5::Digest&gt;::encode):

+        * NetworkProcess/cache/NetworkCacheCoders.h:

+        * NetworkProcess/cache/NetworkCacheDecoder.cpp:

+        (WebKit::NetworkCache::Decoder::Decoder):

+        (WebKit::NetworkCache::Decoder::bufferIsLargeEnoughToContain):

+

+            Reshuffle to avoid sum.

+

+        (WebKit::NetworkCache::Decoder::decodeFixedLengthData):

+        * NetworkProcess/cache/NetworkCacheDecoder.h:

+        (WebKit::NetworkCache::Decoder::bufferSize):

+        (WebKit::NetworkCache::Decoder::currentOffset):

+        (WebKit::NetworkCache::Decoder::length): Deleted.

+        (WebKit::NetworkCache::Decoder::isInvalid): Deleted.

+        (WebKit::NetworkCache::Decoder::markInvalid): Deleted.

+

+            Remove these, they are not really used or needed.

+

</ins><span class="cx"> 2015-06-10  Anders Carlsson  &lt;andersca@apple.com&gt;

</span><span class="cx"> 

</span><span class="cx">         Rewrite WKPluginSiteDataManager using WebsiteDataStore functions

</span></span></pre></div>

<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheCoderscpp"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.cpp (185457 => 185458)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.cpp        2015-06-11 17:01:46 UTC (rev 185457)

+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.cpp        2015-06-11 17:03:02 UTC (rev 185458)

</span><span class="lines">@@ -76,10 +76,8 @@

</span><span class="cx">     }

</span><span class="cx"> 

<span class="cx">     // Before allocating the string, make sure that the decoder buffer is big enough.

</span><del>-    if (!decoder.bufferIsLargeEnoughToContain&lt;char&gt;(length)) {

-        decoder.markInvalid();

</del><ins>+    if (!decoder.bufferIsLargeEnoughToContain&lt;char&gt;(length))

</ins><span class="cx">         return false;

</span><del>-    }

</del><span class="cx"> 

</span><span class="cx">     char* buffer;

</span><span class="cx">     CString string = CString::newUninitialized(length, buffer);

</span><span class="lines">@@ -114,11 +112,9 @@

</span><span class="cx"> static inline bool decodeStringText(Decoder&amp; decoder, uint32_t length, String&amp; result)

</span><span class="cx"> {

<span class="cx">     // Before allocating the string, make sure that the decoder buffer is big enough.

</span><del>-    if (!decoder.bufferIsLargeEnoughToContain&lt;CharacterType&gt;(length)) {

-        decoder.markInvalid();

</del><ins>+    if (!decoder.bufferIsLargeEnoughToContain&lt;CharacterType&gt;(length))

</ins><span class="cx">         return false;

</span><del>-    }

-    

</del><ins>+

</ins><span class="cx">     CharacterType* buffer;

</span><span class="cx">     String string = String::createUninitialized(length, buffer);

</span><span class="cx">     if (!decoder.decodeFixedLengthData(reinterpret_cast&lt;uint8_t*&gt;(buffer), length * sizeof(CharacterType)))

</span><span class="lines">@@ -167,11 +163,7 @@

</span><span class="cx">     if (!decoder.decodeFixedLengthData(data.data(), data.size()))

</span><span class="cx">         return false;

</span><span class="cx">     IPC::ArgumentDecoder argumentDecoder(data.data(), data.size());

</span><del>-    if (!argumentDecoder.decode(certificateInfo)) {

-        decoder.markInvalid();

-        return false;

-    }

-    return true;

</del><ins>+    return argumentDecoder.decode(certificateInfo);

</ins><span class="cx"> }

</span><span class="cx"> 

</span><span class="cx"> void Coder&lt;MD5::Digest&gt;::encode(Encoder&amp; encoder, const MD5::Digest&amp; digest)

</span></span></pre></div>

<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheCodersh"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.h (185457 => 185458)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.h        2015-06-11 17:01:46 UTC (rev 185457)

+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.h        2015-06-11 17:03:02 UTC (rev 185458)

</span><span class="lines">@@ -150,10 +150,8 @@

</span><span class="cx">         // Since we know the total size of the elements, we can allocate the vector in

</span><span class="cx">         // one fell swoop. Before allocating we must however make sure that the decoder buffer

<span class="cx">         // is big enough.

</span><del>-        if (!decoder.bufferIsLargeEnoughToContain&lt;T&gt;(size)) {

-            decoder.markInvalid();

</del><ins>+        if (!decoder.bufferIsLargeEnoughToContain&lt;T&gt;(size))

</ins><span class="cx">             return false;

</span><del>-        }

</del><span class="cx"> 

</span><span class="cx">         Vector&lt;T, inlineCapacity&gt; temp;

</span><span class="cx">         temp.resize(size);

</span><span class="lines">@@ -194,7 +192,6 @@

</span><span class="cx"> 

</span><span class="cx">             if (!tempHashMap.add(key, value).isNewEntry) {

<span class="cx">                 // The hash map already has the specified key, bail.

</span><del>-                decoder.markInvalid();

</del><span class="cx">                 return false;

</span><span class="cx">             }

</span><span class="cx">         }

</span><span class="lines">@@ -228,7 +225,6 @@

</span><span class="cx"> 

</span><span class="cx">             if (!tempHashSet.add(key).isNewEntry) {

<span class="cx">                 // The hash map already has the specified key, bail.

</span><del>-                decoder.markInvalid();

</del><span class="cx">                 return false;

</span><span class="cx">             }

</span><span class="cx">         }

</span></span></pre></div>

<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheDecodercpp"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp (185457 => 185458)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp        2015-06-11 17:01:46 UTC (rev 185457)

+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp        2015-06-11 17:03:02 UTC (rev 185458)

</span><span class="lines">@@ -47,7 +47,7 @@

</span><span class="cx"> 

</span><span class="cx"> bool Decoder::bufferIsLargeEnoughToContain(size_t size) const

</span><span class="cx"> {

</span><del>-    return m_bufferPosition + size &lt;= m_bufferEnd;

</del><ins>+    return size &lt;= static_cast&lt;size_t&gt;(m_bufferEnd - m_bufferPosition);

</ins><span class="cx"> }

</span><span class="cx"> 

</span><span class="cx"> bool Decoder::decodeFixedLengthData(uint8_t* data, size_t size)

</span></span></pre></div>

<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheDecoderh"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h (185457 => 185458)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h        2015-06-11 17:01:46 UTC (rev 185457)

+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h        2015-06-11 17:03:02 UTC (rev 185458)

</span><span class="lines">@@ -42,9 +42,6 @@

</span><span class="cx">     size_t length() const { return m_bufferEnd - m_buffer; }

</span><span class="cx">     size_t currentOffset() const { return m_bufferPosition - m_buffer; }

</span><span class="cx"> 

</span><del>-    bool isInvalid() const { return m_bufferPosition &gt; m_bufferEnd; }

-    void markInvalid() { m_bufferPosition = m_bufferEnd + 1; }

-

</del><span class="cx">     bool verifyChecksum();

</span><span class="cx"> 

</span><span class="cx">     bool decodeFixedLengthData(uint8_t*, size_t);

</span></span></pre>

</div>

</div>

</body>

</html>