Title: [258486] trunk
- Revision
- 258486
- Author
- [email protected]
- Date
- 2020-03-15 19:58:01 -0700 (Sun, 15 Mar 2020)
Log Message
KeyedDecoderGeneric fails to allocate Vector while decoding broken data
https://bugs.webkit.org/show_bug.cgi?id=207324
Reviewed by Darin Adler.
Source/WebCore:
There were three crash bugs in it.
KeyedDecoderGeneric was trying to allocate a buffer without
ensuring the size wouldn't exceed the decoding data size by using
bufferIsLargeEnoughToContain.
It was trying to push an itme into the top dictionary of emtpy
m_dictionaryStack when EndObject tag would appear without the
preceding BeginObject tag.
It was trying to push an item into the top array of empty
m_arrayStack when EndArray tag would appear without the preceding
BeginArray tag.
Tests: TestWebKitAPI: KeyedCoding.DecodeRandomData
* platform/generic/KeyedDecoderGeneric.cpp:
(WebCore::readString):
(WebCore::KeyedDecoderGeneric::KeyedDecoderGeneric):
Check bufferIsLargeEnoughToContain(size) before allocating a Vector with size.
Check if m_dictionaryStack and m_arrayStack are empty.
Tools:
* TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp:
(TestWebKitAPI::generateRandomData): Added.
(TestWebKitAPI::KeyedCoding.DecodeRandomData): Added a new test decoding random data.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (258485 => 258486)
--- trunk/Source/WebCore/ChangeLog 2020-03-16 02:33:48 UTC (rev 258485)
+++ trunk/Source/WebCore/ChangeLog 2020-03-16 02:58:01 UTC (rev 258486)
@@ -1,3 +1,32 @@
+2020-03-15 Fujii Hironori <[email protected]>
+
+ KeyedDecoderGeneric fails to allocate Vector while decoding broken data
+ https://bugs.webkit.org/show_bug.cgi?id=207324
+
+ Reviewed by Darin Adler.
+
+ There were three crash bugs in it.
+
+ KeyedDecoderGeneric was trying to allocate a buffer without
+ ensuring the size wouldn't exceed the decoding data size by using
+ bufferIsLargeEnoughToContain.
+
+ It was trying to push an itme into the top dictionary of emtpy
+ m_dictionaryStack when EndObject tag would appear without the
+ preceding BeginObject tag.
+
+ It was trying to push an item into the top array of empty
+ m_arrayStack when EndArray tag would appear without the preceding
+ BeginArray tag.
+
+ Tests: TestWebKitAPI: KeyedCoding.DecodeRandomData
+
+ * platform/generic/KeyedDecoderGeneric.cpp:
+ (WebCore::readString):
+ (WebCore::KeyedDecoderGeneric::KeyedDecoderGeneric):
+ Check bufferIsLargeEnoughToContain(size) before allocating a Vector with size.
+ Check if m_dictionaryStack and m_arrayStack are empty.
+
2020-03-15 Chris Dumez <[email protected]>
[DRT] InternalSettingsGenerated::resetToConsistentState() may override TestOptions::enableBackForwardCache
Modified: trunk/Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp (258485 => 258486)
--- trunk/Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp 2020-03-16 02:33:48 UTC (rev 258485)
+++ trunk/Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp 2020-03-16 02:58:01 UTC (rev 258486)
@@ -58,6 +58,8 @@
return true;
}
+ if (!decoder.bufferIsLargeEnoughToContain<uint8_t>(size))
+ return false;
Vector<uint8_t> buffer(size);
if (!decoder.decodeFixedLengthData(buffer.data(), size))
return false;
@@ -108,6 +110,9 @@
ok = decoder.decode(size);
if (!ok)
break;
+ ok = decoder.bufferIsLargeEnoughToContain<uint8_t>(size);
+ if (!ok)
+ break;
Vector<uint8_t> buffer(size);
ok = decoder.decodeFixedLengthData(buffer.data(), size);
if (!ok)
@@ -159,6 +164,8 @@
}
case KeyedEncoderGeneric::Type::EndObject:
m_dictionaryStack.removeLast();
+ if (m_dictionaryStack.isEmpty())
+ ok = false;
break;
case KeyedEncoderGeneric::Type::BeginArray: {
ok = readString(decoder, key);
@@ -170,6 +177,9 @@
break;
}
case KeyedEncoderGeneric::Type::BeginArrayElement: {
+ ok = !m_arrayStack.isEmpty();
+ if (!ok)
+ break;
auto newDictionary = makeUnique<Dictionary>();
m_dictionaryStack.append(newDictionary.get());
m_arrayStack.last()->append(WTFMove(newDictionary));
@@ -177,8 +187,13 @@
}
case KeyedEncoderGeneric::Type::EndArrayElement:
m_dictionaryStack.removeLast();
+ if (m_dictionaryStack.isEmpty())
+ ok = false;
break;
case KeyedEncoderGeneric::Type::EndArray:
+ ok = !m_arrayStack.isEmpty();
+ if (!ok)
+ break;
m_arrayStack.removeLast();
break;
}
Modified: trunk/Tools/ChangeLog (258485 => 258486)
--- trunk/Tools/ChangeLog 2020-03-16 02:33:48 UTC (rev 258485)
+++ trunk/Tools/ChangeLog 2020-03-16 02:58:01 UTC (rev 258486)
@@ -1,3 +1,14 @@
+2020-03-15 Fujii Hironori <[email protected]>
+
+ KeyedDecoderGeneric fails to allocate Vector while decoding broken data
+ https://bugs.webkit.org/show_bug.cgi?id=207324
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp:
+ (TestWebKitAPI::generateRandomData): Added.
+ (TestWebKitAPI::KeyedCoding.DecodeRandomData): Added a new test decoding random data.
+
2020-03-15 Yusuke Suzuki <[email protected]>
Should not use variable-length-array (VLA)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp (258485 => 258486)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp 2020-03-16 02:33:48 UTC (rev 258485)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp 2020-03-16 02:58:01 UTC (rev 258486)
@@ -28,6 +28,7 @@
#include <WebCore/KeyedCoding.h>
#include <WebCore/SharedBuffer.h>
#include <cstdint>
+#include <cstdlib>
#include <wtf/text/WTFString.h>
namespace TestWebKitAPI {
@@ -290,4 +291,23 @@
EXPECT_TRUE(success);
EXPECT_EQ(false, boolValue);
}
+
+static Vector<uint8_t> generateRandomData()
+{
+ Vector<uint8_t> data;
+ for (auto i = 0; i < 256; ++i)
+ data.append(std::rand() / (RAND_MAX + 1.0) * std::numeric_limits<uint8_t>::max());
+ return data;
}
+
+TEST(KeyedCoding, DecodeRandomData)
+{
+ std::srand(0);
+ for (auto i = 0; i < 10; ++i) {
+ auto data = ""
+ // Don't crash.
+ WebCore::KeyedDecoder::decoder(data.data(), data.size());
+ }
+}
+
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
