Changeset 280900 in webkit

Timestamp:

Aug 11, 2021, 3:43:09 AM (5 years ago)

Author:

Adrian Perez de Castro

Message:

Merge r274776 - Better validate JSArrays in AudioWorkletProcessor
​https://bugs.webkit.org/show_bug.cgi?id=223548

Reviewed by Geoffrey Garen.

Better validate JSArrays in AudioWorkletProcessor. Replaces debug assertions with runtime
checks for robustness.

• Modules/webaudio/AudioWorkletProcessor.cpp:

(WebCore::copyDataFromBusesToJSArray):
(WebCore::copyDataFromParameterMapToJSObject):
(WebCore::zeroJSArray):
(WebCore::AudioWorkletProcessor::buildJSArguments):

Location:

releases/WebKitGTK/webkit-2.32/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/webaudio/AudioWorkletProcessor.cpp (modified) (3 diffs)

releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-22  Chris Dumez  <cdumez@apple.com>
+
+        Better validate JSArrays in AudioWorkletProcessor
+        https://bugs.webkit.org/show_bug.cgi?id=223548
+
+        Reviewed by Geoffrey Garen.
+
+        Better validate JSArrays in AudioWorkletProcessor. Replaces debug assertions with runtime
+        checks for robustness.
+
+        * Modules/webaudio/AudioWorkletProcessor.cpp:
+        (WebCore::copyDataFromBusesToJSArray):
+        (WebCore::copyDataFromParameterMapToJSObject):
+        (WebCore::zeroJSArray):
+        (WebCore::AudioWorkletProcessor::buildJSArguments):
+
 2021-03-19  Rob Buis  <rbuis@igalia.com>
 

releases/WebKitGTK/webkit-2.32/Source/WebCore/Modules/webaudio/AudioWorkletProcessor.cpp

@@ -66 +66 @@
 }
 
-static void forEachChannelDataJSArray(JSGlobalObject& globalObject, JSArray& jsArray, const Function<void(unsigned busIndex, unsigned channelIndex, JSFloat32Array& channelData)>& apply)
-{
-    for (unsigned busIndex = 0, busCount = jsArray.length(); busIndex < busCount; ++busIndex) {
-        auto* channelsArray = jsCast<JSArray*>(jsArray.getIndex(&globalObject, busIndex));
-        for (unsigned channelIndex = 0, channelCount = channelsArray->length(); channelIndex < channelCount; ++channelIndex)
-            apply(busIndex, channelIndex, *jsCast<JSFloat32Array*>(channelsArray->getIndex(&globalObject, channelIndex)));
-    }
-}
-
 static JSFloat32Array* constructJSFloat32Array(JSGlobalObject& globalObject, unsigned length, const float* data = nullptr)
 {
@@ -144 +135 @@
 }
 
-static void copyDataFromBusesToJSArray(JSGlobalObject& globalObject, const Vector<RefPtr<AudioBus>>& buses, JSArray& jsArray)
-{
-    forEachChannelDataJSArray(globalObject, jsArray, [&](unsigned busIndex, unsigned channelIndex, JSFloat32Array& channelData) {
-        auto* channel = buses[busIndex]->channel(channelIndex);
-        ASSERT(channelData.length() == channel->length());
-        memcpy(channelData.typedVector(), channel->mutableData(), sizeof(float) * channel->length());
-    });
-}
-
-static void copyDataFromParameterMapToJSObject(VM& vm, JSGlobalObject& globalObject, const HashMap<String, std::unique_ptr<AudioFloatArray>>& paramValuesMap, JSObject& jsObject)
-{
+static bool copyDataFromBusesToJSArray(VM& vm, JSGlobalObject& globalObject, const Vector<RefPtr<AudioBus>>& buses, JSArray* jsArray)
+{
+    if (!jsArray)
+        return false;
+
+    for (size_t busIndex = 0; busIndex < buses.size(); ++busIndex) {
+        auto& bus = buses[busIndex];
+        auto* jsChannelsArray = jsDynamicCast<JSArray*>(vm, jsArray->getIndex(&globalObject, busIndex));
+        unsigned numberOfChannels = busChannelCount(bus.get());
+        if (!jsChannelsArray || jsChannelsArray->length() != numberOfChannels)
+            return false;
+        for (unsigned channelIndex = 0; channelIndex < numberOfChannels; ++channelIndex) {
+            auto* channel = bus->channel(channelIndex);
+            auto* jsChannelArray = jsDynamicCast<JSFloat32Array*>(vm, jsChannelsArray->getIndex(&globalObject, channelIndex));
+            if (!jsChannelArray || jsChannelArray->length() != channel->length())
+                return false;
+            memcpy(jsChannelArray->typedVector(), channel->mutableData(), sizeof(float) * jsChannelArray->length());
+        }
+    }
+    return true;
+}
+
+static bool copyDataFromParameterMapToJSObject(VM& vm, JSGlobalObject& globalObject, const HashMap<String, std::unique_ptr<AudioFloatArray>>& paramValuesMap, JSObject* jsObject)
+{
+    if (!jsObject)
+        return false;
+
     for (auto& pair : paramValuesMap) {
-        auto* jsTypedArray = jsCast<JSFloat32Array*>(jsObject.get(&globalObject, Identifier::fromString(vm, pair.key)));
-        ASSERT(pair.value->size() >= jsTypedArray->length());
-        memcpy(jsTypedArray->typedVector(), pair.value->data(), sizeof(float) * jsTypedArray->length());
-    }
-}
-
-template<typename T>
-static bool busTopologyMatchesJSArray(JSGlobalObject& globalObject, const Vector<T>& buses, JSArray* jsArray)
-{
-    if (!jsArray)
-        return false;
-
-    ASSERT_WITH_MESSAGE(jsArray->length() == buses.size(), "Number of inputs/outputs cannot change after construction");
-
-    for (unsigned i = 0; i < buses.size(); ++i) {
-        auto& bus = buses[i];
-        auto* channelsArray = jsCast<JSArray*>(jsArray->getIndex(&globalObject, i));
-        unsigned numberOfChannels = busChannelCount(bus.get());
-        if (channelsArray->length() != numberOfChannels)
-            return false;
-
-        for (unsigned j = 0; j < numberOfChannels; ++j) {
-            auto* channel = bus->channel(j);
-            auto* jsChannelData = jsCast<JSFloat32Array*>(channelsArray->getIndex(&globalObject, j));
-            if (jsChannelData->length() != channel->length())
-                return false;
-        }
-    }
-
-    return true;
-}
-
-static bool parameterMapTopologyMatchesJSObject(VM& vm, JSGlobalObject& globalObject, const HashMap<String, std::unique_ptr<AudioFloatArray>>& paramValuesMap, JSObject* jsObject)
-{
-    if (!jsObject)
-        return false;
-
-    for (auto& pair : paramValuesMap) {
-        auto* jsTypedArray = jsCast<JSFloat32Array*>(jsObject->get(&globalObject, Identifier::fromString(vm, pair.key)));
+        auto* jsTypedArray = jsDynamicCast<JSFloat32Array*>(vm, jsObject->get(&globalObject, Identifier::fromString(vm, pair.key)));
+        if (!jsTypedArray)
+            return false;
         unsigned expectedLength = pair.value->containsConstantValue() ? 1 : pair.value->size();
         if (jsTypedArray->length() != expectedLength)
             return false;
-    }
-
+        memcpy(jsTypedArray->typedVector(), pair.value->data(), sizeof(float) * jsTypedArray->length());
+    }
     return true;
 }
 
-static void zeroJSArray(JSGlobalObject& globalObject, JSArray& jsArray)
-{
-    forEachChannelDataJSArray(globalObject, jsArray, [](unsigned, unsigned, JSFloat32Array& channelData) {
-        memset(channelData.typedVector(), 0, sizeof(float) * channelData.length());
-    });
+static bool zeroJSArray(VM& vm, JSGlobalObject& globalObject, const Vector<Ref<AudioBus>>& outputs, JSArray* jsArray)
+{
+    if (!jsArray)
+        return false;
+
+    for (size_t busIndex = 0; busIndex < outputs.size(); ++busIndex) {
+        auto& bus = outputs[busIndex];
+        auto* jsChannelsArray = jsDynamicCast<JSArray*>(vm, jsArray->getIndex(&globalObject, busIndex));
+        unsigned numberOfChannels = busChannelCount(bus.get());
+        if (!jsChannelsArray || jsChannelsArray->length() != numberOfChannels)
+            return false;
+        for (unsigned channelIndex = 0; channelIndex < numberOfChannels; ++channelIndex) {
+            auto* channel = bus->channel(channelIndex);
+            auto* jsChannelArray = jsDynamicCast<JSFloat32Array*>(vm, jsChannelsArray->getIndex(&globalObject, channelIndex));
+            if (!jsChannelArray || jsChannelArray->length() != channel->length())
+                return false;
+            memset(jsChannelArray->typedVector(), 0, sizeof(float) * jsChannelArray->length());
+        }
+    }
+    return true;
 }
 
@@ -231 +217 @@
 {
     // For performance reasons, we cache the arrays passed to JS and reconstruct them only when the topology changes.
-    if (busTopologyMatchesJSArray(globalObject, inputs, toJSArray(m_jsInputs)))
-        copyDataFromBusesToJSArray(globalObject, inputs, *toJSArray(m_jsInputs));
-    else
+    if (!copyDataFromBusesToJSArray(vm, globalObject, inputs, toJSArray(m_jsInputs)))
         m_jsInputs = { constructFrozenJSArray(vm, globalObject, inputs, ShouldPopulateWithBusData::Yes) };
     args.append(m_jsInputs);
 
-    if (busTopologyMatchesJSArray(globalObject, outputs, toJSArray(m_jsOutputs)))
-        zeroJSArray(globalObject, *toJSArray(m_jsOutputs));
-    else
+    if (!zeroJSArray(vm, globalObject, outputs, toJSArray(m_jsOutputs)))
         m_jsOutputs = { constructFrozenJSArray(vm, globalObject, outputs, ShouldPopulateWithBusData::No) };
     args.append(m_jsOutputs);
 
-    if (parameterMapTopologyMatchesJSObject(vm, globalObject, paramValuesMap, toJSObject(m_jsParamValues)))
-        copyDataFromParameterMapToJSObject(vm, globalObject, paramValuesMap, *toJSObject(m_jsParamValues));
-    else
+    if (!copyDataFromParameterMapToJSObject(vm, globalObject, paramValuesMap, toJSObject(m_jsParamValues)))
         m_jsParamValues = { constructFrozenKeyValueObject(vm, globalObject, paramValuesMap) };
     args.append(m_jsParamValues);