Changeset 258486 in webkit

Timestamp:

Mar 15, 2020 7:58:01 PM (4 years ago)

Author:

Fujii Hironori

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.

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp (modified) (5 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-03-15  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        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  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/generic/KeyedDecoderGeneric.cpp

@@ -59 +59 @@
     }
 
+    if (!decoder.bufferIsLargeEnoughToContain<uint8_t>(size))
+        return false;
     Vector<uint8_t> buffer(size);
     if (!decoder.decodeFixedLengthData(buffer.data(), size))
@@ -109 +111 @@
             if (!ok)
                 break;
+            ok = decoder.bufferIsLargeEnoughToContain<uint8_t>(size);
+            if (!ok)
+                break;
             Vector<uint8_t> buffer(size);
             ok = decoder.decodeFixedLengthData(buffer.data(), size);
@@ -160 +165 @@
         case KeyedEncoderGeneric::Type::EndObject:
             m_dictionaryStack.removeLast();
+            if (m_dictionaryStack.isEmpty())
+                ok = false;
             break;
         case KeyedEncoderGeneric::Type::BeginArray: {
@@ -171 +178 @@
         }
         case KeyedEncoderGeneric::Type::BeginArrayElement: {
+            ok = !m_arrayStack.isEmpty();
+            if (!ok)
+                break;
             auto newDictionary = makeUnique<Dictionary>();
             m_dictionaryStack.append(newDictionary.get());
@@ -178 +188 @@
         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;

trunk/Tools/ChangeLog

@@ +1 @@
+2020-03-15  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        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  <ysuzuki@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebCore/KeyedCoding.cpp

@@ -29 +29 @@
 #include <WebCore/SharedBuffer.h>
 #include <cstdint>
+#include <cstdlib>
 #include <wtf/text/WTFString.h>
 
@@ -291 +292 @@
     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 = generateRandomData();
+        // Don't crash.
+        WebCore::KeyedDecoder::decoder(data.data(), data.size());
+    }
+}
+
+}