Title: [185458] trunk/Source/WebKit2
- Revision
- 185458
- Author
- [email protected]
- Date
- 2015-06-11 10:03:02 -0700 (Thu, 11 Jun 2015)
Log Message
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<CString>::decode):
(WebKit::NetworkCache::decodeStringText):
(WebKit::NetworkCache::Coder<WebCore::CertificateInfo>::decode):
(WebKit::NetworkCache::Coder<MD5::Digest>::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.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (185457 => 185458)
--- 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)
@@ -1,3 +1,39 @@
+2015-06-11 Antti Koivisto <[email protected]>
+
+ 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<CString>::decode):
+ (WebKit::NetworkCache::decodeStringText):
+ (WebKit::NetworkCache::Coder<WebCore::CertificateInfo>::decode):
+ (WebKit::NetworkCache::Coder<MD5::Digest>::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.
+
2015-06-10 Anders Carlsson <[email protected]>
Rewrite WKPluginSiteDataManager using WebsiteDataStore functions
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.cpp (185457 => 185458)
--- 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)
@@ -76,10 +76,8 @@
}
// Before allocating the string, make sure that the decoder buffer is big enough.
- if (!decoder.bufferIsLargeEnoughToContain<char>(length)) {
- decoder.markInvalid();
+ if (!decoder.bufferIsLargeEnoughToContain<char>(length))
return false;
- }
char* buffer;
CString string = CString::newUninitialized(length, buffer);
@@ -114,11 +112,9 @@
static inline bool decodeStringText(Decoder& decoder, uint32_t length, String& result)
{
// Before allocating the string, make sure that the decoder buffer is big enough.
- if (!decoder.bufferIsLargeEnoughToContain<CharacterType>(length)) {
- decoder.markInvalid();
+ if (!decoder.bufferIsLargeEnoughToContain<CharacterType>(length))
return false;
- }
-
+
CharacterType* buffer;
String string = String::createUninitialized(length, buffer);
if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(buffer), length * sizeof(CharacterType)))
@@ -167,11 +163,7 @@
if (!decoder.decodeFixedLengthData(data.data(), data.size()))
return false;
IPC::ArgumentDecoder argumentDecoder(data.data(), data.size());
- if (!argumentDecoder.decode(certificateInfo)) {
- decoder.markInvalid();
- return false;
- }
- return true;
+ return argumentDecoder.decode(certificateInfo);
}
void Coder<MD5::Digest>::encode(Encoder& encoder, const MD5::Digest& digest)
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheCoders.h (185457 => 185458)
--- 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)
@@ -150,10 +150,8 @@
// Since we know the total size of the elements, we can allocate the vector in
// one fell swoop. Before allocating we must however make sure that the decoder buffer
// is big enough.
- if (!decoder.bufferIsLargeEnoughToContain<T>(size)) {
- decoder.markInvalid();
+ if (!decoder.bufferIsLargeEnoughToContain<T>(size))
return false;
- }
Vector<T, inlineCapacity> temp;
temp.resize(size);
@@ -194,7 +192,6 @@
if (!tempHashMap.add(key, value).isNewEntry) {
// The hash map already has the specified key, bail.
- decoder.markInvalid();
return false;
}
}
@@ -228,7 +225,6 @@
if (!tempHashSet.add(key).isNewEntry) {
// The hash map already has the specified key, bail.
- decoder.markInvalid();
return false;
}
}
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.cpp (185457 => 185458)
--- 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)
@@ -47,7 +47,7 @@
bool Decoder::bufferIsLargeEnoughToContain(size_t size) const
{
- return m_bufferPosition + size <= m_bufferEnd;
+ return size <= static_cast<size_t>(m_bufferEnd - m_bufferPosition);
}
bool Decoder::decodeFixedLengthData(uint8_t* data, size_t size)
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheDecoder.h (185457 => 185458)
--- 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)
@@ -42,9 +42,6 @@
size_t length() const { return m_bufferEnd - m_buffer; }
size_t currentOffset() const { return m_bufferPosition - m_buffer; }
- bool isInvalid() const { return m_bufferPosition > m_bufferEnd; }
- void markInvalid() { m_bufferPosition = m_bufferEnd + 1; }
-
bool verifyChecksum();
bool decodeFixedLengthData(uint8_t*, size_t);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
