Attachment 346700 Details for Bug 188376 – Patch

[patch] Patch

bug-188376-20180807102505.patch (text/plain), 17.08 KB, created by Charlie Turner on 2018-08-07 02:25:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Charlie Turner

Created: 2018-08-07 02:25:06 PDT

Size: 17.08 KB

patch

obsolete

>Subversion Revision: 234641
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0c9ea71d8117106efdd24d081ea048283897029f..7f5ec175300a0cca282dafc516a16870a1abdefc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+2018-08-07  Charlie Turner  <cturner@igalia.com>
>+
>+        Add CENC sanitization
>+        https://bugs.webkit.org/show_bug.cgi?id=188376
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adds support for sanitizing the CENC initialization
>+        data, to ensure there are no obviously bogus values in the
>+        untrusted input from generateRequest, see
>+        https://www.w3.org/TR/encrypted-media/#dom-mediakeysession-generaterequest
>+        for more details on sanitization.
>+
>+        Tested by imported/w3c/web-platform-tests/encrypted-media/clearkey-generate-request-disallowed-input.https.html
>+
>+        * Modules/encryptedmedia/InitDataRegistry.cpp: The diff looks a
>+        bit wonky because I had to reorder extractKeyIDsCenc to come
>+        before sanitizeCenc.
>+        (WebCore::extractKeyIDsCenc): Added implementation, a nullopt
>+        return value here indicates the parsing found an error in the box
>+        values.
>+        (WebCore::sanitizeCenc): Added implementation, if the box can be
>+        parsed, return a copy of the buffer as before, otherwise an error
>+        value.
>+        * Sources.txt: Add the new PSSH box type.
>+        * platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:
>+        Added, parsing methods come from ISO/IEC 23001-7-2016 Section
>+        8.1.1.
>+        (WebCore::ISOProtectionSystemSpecificHeaderBox::parse): Ditto.
>+        * platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h: Ditto.
>+        (WebCore::ISOProtectionSystemSpecificHeaderBox::boxTypeName): Ditto.
>+        (WebCore::ISOProtectionSystemSpecificHeaderBox::systemID const): Ditto.
>+        (WebCore::ISOProtectionSystemSpecificHeaderBox::keyIDs const):
>+        Ditto.
>+        (WebCore::ISOProtectionSystemSpecificHeaderBox::data const): Ditto.
>+
> 2018-08-04  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Add CEReactions=NotNeeded for reactions only needed for customized builtins
>diff --git a/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp b/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp
>index 1d67ed66bc32a4e06b3c67955b54bbdcc46795d8..a7c7143b3a54a275ec3909a0474e7c7042d8773e 100644
>--- a/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp
>+++ b/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp
>@@ -28,6 +28,8 @@
> 
> #if ENABLE(ENCRYPTED_MEDIA)
> 
>+#include "ISOProtectionSystemSpecificHeaderBox.h"
>+#include <JavaScriptCore/DataView.h>
> #include "NotImplemented.h"
> #include "SharedBuffer.h"
> #include <wtf/JSONValues.h>
>@@ -37,6 +39,10 @@
> 
> namespace WebCore {
> 
>+namespace {
>+    const uint32_t kCencMaxBoxSize = 64 * KB;
>+}
>+
> static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsKeyids(const SharedBuffer& buffer)
> {
>     // 1. Format
>@@ -92,20 +98,45 @@ static RefPtr<SharedBuffer> sanitizeKeyids(const SharedBuffer& buffer)
>     return SharedBuffer::create(jsonData.data(), jsonData.length());
> }
> 
>-static RefPtr<SharedBuffer> sanitizeCenc(const SharedBuffer& buffer)
>+static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsCenc(const SharedBuffer& buffer)
> {
>     // 4. Common SystemID and PSSH Box Format
>     // https://w3c.github.io/encrypted-media/format-registry/initdata/cenc.html#common-system
>-    notImplemented();
>-    return buffer.copy();
>+    if (buffer.size() >= kCencMaxBoxSize)
>+        return std::nullopt;
>+
>+    unsigned offset = 0;
>+    Vector<Ref<SharedBuffer>> keyIDs;
>+
>+    auto view = JSC::DataView::create(buffer.tryCreateArrayBuffer(), offset, buffer.size());
>+    while (auto optionalBoxType = ISOBox::peekBox(view, offset)) {
>+        auto& boxTypeName = optionalBoxType.value().first;
>+        auto& boxSize = optionalBoxType.value().second;
>+
>+        if (boxTypeName != ISOProtectionSystemSpecificHeaderBox::boxTypeName() || boxSize > buffer.size())
>+            return std::nullopt;
>+
>+        ISOProtectionSystemSpecificHeaderBox psshBox;
>+        if (!psshBox.read(view, offset))
>+            return std::nullopt;
>+
>+        for (auto& value : psshBox.keyIDs()) {
>+            Ref<SharedBuffer> keyIDBuffer = SharedBuffer::create(WTFMove(value));
>+            keyIDs.append(WTFMove(keyIDBuffer));
>+        }
>+    }
>+
>+    return keyIDs;
> }
> 
>-static std::optional<Vector<Ref<SharedBuffer>>> extractKeyIDsCenc(const SharedBuffer&)
>+static RefPtr<SharedBuffer> sanitizeCenc(const SharedBuffer& buffer)
> {
>     // 4. Common SystemID and PSSH Box Format
>     // https://w3c.github.io/encrypted-media/format-registry/initdata/cenc.html#common-system
>-    notImplemented();
>-    return std::nullopt;
>+    if (!extractKeyIDsCenc(buffer))
>+        return nullptr;
>+
>+    return buffer.copy();
> }
> 
> static RefPtr<SharedBuffer> sanitizeWebM(const SharedBuffer& buffer)
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index dfd1b5207c1377948bfcab4bfe204d40e2aa8278..7941575e476a7b5782c67cbc7acb354e49991f60 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -1700,6 +1700,7 @@ platform/graphics/gpu/GPUTextureDescriptor.cpp
> platform/graphics/iso/ISOBox.cpp
> platform/graphics/iso/ISOOriginalFormatBox.cpp
> platform/graphics/iso/ISOProtectionSchemeInfoBox.cpp
>+platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp
> platform/graphics/iso/ISOSchemeInformationBox.cpp
> platform/graphics/iso/ISOSchemeTypeBox.cpp
> platform/graphics/iso/ISOTrackEncryptionBox.cpp
>diff --git a/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp b/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..394fdfc6c0fa383cdfe4ebf4fd14875840f6916e
>--- /dev/null
>+++ b/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp
>@@ -0,0 +1,88 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>+ * THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "ISOProtectionSystemSpecificHeaderBox.h"
>+
>+#include <JavaScriptCore/DataView.h>
>+
>+using JSC::DataView;
>+
>+namespace WebCore {
>+
>+bool ISOProtectionSystemSpecificHeaderBox::parse(DataView& view, unsigned& offset)
>+{
>+    if (!ISOFullBox::parse(view, offset))
>+        return false;
>+
>+    // ISO/IEC 23001-7-2016 Section 8.1.1
>+    auto buffer = view.possiblySharedBuffer();
>+    if (!buffer)
>+        return false;
>+    auto systemID = buffer->slice(offset, offset + 16);
>+    if (!systemID)
>+        return false;
>+
>+    offset += 16;
>+
>+    m_systemID.resize(16);
>+    memcpy(m_systemID.data(), systemID->data(), 16);
>+
>+    if (m_version) {
>+        uint32_t keyIDCount = 0;
>+        if (!checkedRead<uint32_t>(keyIDCount, view, offset, BigEndian))
>+            return false;
>+        if (buffer->byteLength() - offset < keyIDCount * 16)
>+            return false;
>+        m_keyIDs.resize(keyIDCount);
>+        for (unsigned keyID = 0; keyID < keyIDCount; keyID++) {
>+            auto& currentKeyID = m_keyIDs[keyID];
>+            currentKeyID.resize(16);
>+            auto parsedKeyID = buffer->slice(offset, offset + 16);
>+            if (!parsedKeyID)
>+                return false;
>+            offset += 16;
>+            memcpy(currentKeyID.data(), parsedKeyID->data(), 16);
>+        }
>+    }
>+
>+    uint32_t dataSize = 0;
>+    if (!checkedRead<uint32_t>(dataSize, view, offset, BigEndian))
>+        return false;
>+    if (buffer->byteLength() - offset < dataSize)
>+        return false;
>+    auto parsedData = buffer->slice(offset, offset + dataSize);
>+    if (!parsedData)
>+        return false;
>+
>+    offset += dataSize;
>+
>+    m_data.resize(dataSize);
>+    memcpy(m_data.data(), parsedData->data(), dataSize);
>+
>+    return true;
>+}
>+
>+}
>diff --git a/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h b/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..83b5d04e72f5641ab9e456ca7e24f846d825d801
>--- /dev/null
>+++ b/Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.h
>@@ -0,0 +1,52 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>+ * THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#pragma once
>+
>+#include "ISOBox.h"
>+#include "ISOProtectionSystemSpecificHeaderBox.h"
>+
>+namespace WebCore {
>+
>+class ISOProtectionSystemSpecificHeaderBox : public ISOFullBox {
>+public:
>+    using KeyID = Vector<uint8_t>;
>+    static FourCC boxTypeName() { return "pssh"; }
>+
>+    Vector<uint8_t> systemID() const { return m_systemID; }
>+    Vector<KeyID> keyIDs() const { return m_keyIDs; }
>+    Vector<uint8_t> data() const { return m_data; }
>+
>+protected:
>+    bool parse(JSC::DataView&, unsigned& offset) override;
>+
>+    Vector<uint8_t> m_systemID;
>+    Vector<KeyID> m_keyIDs;
>+    Vector<uint8_t> m_data;
>+};
>+
>+}
>+
>+SPECIALIZE_TYPE_TRAITS_ISOBOX(ISOProtectionSystemSpecificHeaderBox)
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 7989a883d956d87048873d94c320e6b58a89de75..76d7c5b9dd1034f558603d3090b55320c5aec1e1 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-08-07  Charlie Turner  <cturner@igalia.com>
>+
>+        Add CENC sanitization
>+        https://bugs.webkit.org/show_bug.cgi?id=188376
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/encrypted-media/scripts/generate-request-disallowed-input.js:
>+        (runTest): Fixed an incorrect test failure message, added more
>+        invalid init data tests to check the new box parsing methods in
>+        WebCore.
>+
> 2018-08-06  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         HTML parser should execute custom element reactions for setting attributes immediately after creating a custom element
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/encrypted-media/scripts/generate-request-disallowed-input.js b/LayoutTests/imported/w3c/web-platform-tests/encrypted-media/scripts/generate-request-disallowed-input.js
>index 8b883ccacff17468f98df5d9d704c494abbac51d..001c00e25bd93a76a1ba1c9096a66c1db93f1471 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/encrypted-media/scripts/generate-request-disallowed-input.js
>+++ b/LayoutTests/imported/w3c/web-platform-tests/encrypted-media/scripts/generate-request-disallowed-input.js
>@@ -24,7 +24,7 @@ function runTest(config,qualifier) {
>         0xAC, 0xE3, 0x3C, 0x1E, 0x52, 0xE2, 0xFB, 0x4B,
>         0x00, 0x00, 0x00, 0x00                           // datasize
>     ]);
>-    push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid initdata (invalid pssh)');
>+    push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid initdata (size too large)');
> 
>     // Invalid data as type = 'psss'.
>     initData = new Uint8Array([
>@@ -38,6 +38,58 @@ function runTest(config,qualifier) {
>     ]);
>     push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid initdata (not pssh)');
> 
>+    initData = new Uint8Array([
>+        0x00, 0x00, 0x00, 0x44, 0x70, 0x73, 0x73, 0x68, // BMFF box header (68 bytes, 'pssh')
>+        0x01, 0x00, 0x00, 0x00,                         // Full box header (version = 1, flags = 0)
>+        0x10, 0x77, 0xef, 0xec, 0xc0, 0xb2, 0x4d, 0x02, // SystemID
>+        0xac, 0xe3, 0x3c, 0x1e, 0x52, 0xe2, 0xfb, 0x4b,
>+        0x00, 0x00, 0x00, 0x04,                         // KID_count (4) (incorrect)
>+        0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, // First KID ("0123456789012345")
>+        0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35,
>+        0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // Second KID ("ABCDEFGHIJKLMNOP")
>+        0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50,
>+        0x00, 0x00, 0x00, 0x00                          // Size of Data (0)
>+    ]);
>+    push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid key id length (4 instead of 2)');
>+
>+    initData = new Uint8Array([
>+        0x00, 0x00, 0x00, 0x54, 0x70, 0x73, 0x73, 0x68, // BMFF box header (84 bytes, 'pssh')
>+        0x01, 0x00, 0x00, 0x00,                         // Full box header (version = 1, flags = 0)
>+        0x10, 0x77, 0xef, 0xec, 0xc0, 0xb2, 0x4d, 0x02, // SystemID
>+        0xac, 0xe3, 0x3c, 0x1e, 0x52, 0xe2, 0xfb, 0x4b,
>+        0x00, 0x00, 0x00, 0x02,                         // KID_count (2)
>+        0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, // First KID ("0123456789012345")
>+        0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35,
>+        0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, // Second KID ("ABCDEFGHIJKLMNOP")
>+        0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50,
>+        0x00, 0x00, 0x00, 0x20,                         // Size of Data (32, incorrect)
>+        0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48,
>+        0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50
>+    ]);
>+    push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid data size (32 instead of 16)');
>+
>+    // Invalid size in a second PSSH blob
>+    // The CENC format allows multiple concatenated boxes, see https://www.w3.org/TR/eme-initdata-cenc/#format
>+    initData = new Uint8Array([
>+        0x00, 0x00, 0x00, 0x20,                          // size = 32
>+        0x70, 0x73, 0x73, 0x68,                          // 'pssh'
>+        0x00,                                            // version = 0
>+        0x00, 0x00, 0x00,                                // flags
>+        0x10, 0x77, 0xEF, 0xEC, 0xC0, 0xB2, 0x4D, 0x02,  // Common SystemID
>+        0xAC, 0xE3, 0x3C, 0x1E, 0x52, 0xE2, 0xFB, 0x4B,
>+        0x00, 0x00, 0x00, 0x00,                          // datasize
>+
>+        0x00, 0x00, 0x00, 0x10,                          // size = 16 (incorrect)
>+        0x70, 0x73, 0x73, 0x68,                          // 'pssh'
>+        0x00,                                            // version = 0
>+        0x00, 0x00, 0x00,                                // flags
>+        0x10, 0x77, 0xEF, 0xEC, 0xC0, 0xB2, 0x4D, 0x02,  // Common SystemID
>+        0xAC, 0xE3, 0x3C, 0x1E, 0x52, 0xE2, 0xFB, 0x4B,
>+        0x00, 0x00, 0x00, 0x00                           // datasize
>+
>+    ]);
>+    push_test(config.keysystem, 'cenc', initData, testnamePrefix( qualifier, config.keysystem ) + ', temporary, cenc, invalid initdata (second box has incorrect size)');
>+
>     // Valid key ID size must be at least 1 character for keyids.
>     keyId = new Uint8Array(0);
>     initData = stringToUint8Array(createKeyIDs(keyId));

Actions: View | Formatted Diff | Diff

Attachments on bug 188376: 346700 | 346703 | 346706 | 346762