Security hardening for SincResampler

https://bugs.webkit.org/show_bug.cgi?id=261317
rdar://105650262

Reviewed by David Kilzer and Darin Adler.

Do security hardening for SincResampler as we have evidence that we're getting
the logic wrong in some cases and doing a heap-buffer overflow WRITE.

This patch updates SincResampler to use `std::span<float>` instead of `float*` and
to leverage new memcpySpans() / memsetSpan() functions
I added to WTF.

This had several benefits:
- Using std::span means we don't lose tracks of our buffer bounds so we can do
  extra bounds checks.
- We benefit from std::span's bounds checks too which are already enabled on trunk
  via `-D_LIBCPP_ENABLE_ASSERTIONS=1`. Those checks apply to subspan() and operator[]
  in particular, both of which are used by SincResampler.

* Source/WTF/WTF.xcodeproj/project.pbxproj:
* Source/WTF/wtf/Algorithms.h:.
(WTF::memcpySpans):
(WTF::memsetSpan):
* Source/WebCore/platform/audio/AudioArray.h:
(WebCore::AudioArray::toSpan):
(WebCore::AudioArray::toSpan const):
* Source/WebCore/platform/audio/AudioBus.cpp:
(WebCore::AudioBus::createBySampleRateConverting):
* Source/WebCore/platform/audio/AudioChannel.h:
* Source/WebCore/platform/audio/MultiChannelResampler.cpp:
(WebCore::MultiChannelResampler::process):
(WebCore::MultiChannelResampler::provideInputForChannel):
* Source/WebCore/platform/audio/MultiChannelResampler.h:
* Source/WebCore/platform/audio/SincResampler.cpp:
(WebCore::SincResampler::SincResampler):
(WebCore::SincResampler::updateRegions):
(WebCore::SincResampler::processBuffer):
(WebCore::SincResampler::process):
* Source/WebCore/platform/audio/SincResampler.h:

Canonical link: https://commits.webkit.org/265870.537@safari-7616-branch

Source/WTF/wtf/Algorithms.h

@@ -25,6 +25,10 @@
 
 #pragma once
 
+#include <cstring>
+#include <span>
+#include <wtf/Assertions.h>
+
 namespace WTF {
 
 template<typename ContainerType, typename ForEachFunction>
@@ -54,5 +58,22 @@ bool allOf(ContainerType&& container, AllOfFunction allOfFunction)
     return true;
 }
 
+template<typename T, typename U>
+void memcpySpan(std::span<T> destination, std::span<U> source)
+{
+    RELEASE_ASSERT(destination.size() == source.size());
+    static_assert(sizeof(T) == sizeof(U));
+    memcpy(destination.data(), source.data(), destination.size() * sizeof(T));
 }
 
+template<typename T>
+void memsetSpan(std::span<T> destination, uint8_t byte)
+{
+    memset(destination.data(), byte, destination.size() * sizeof(T));
+}
+
+} // namespace WTF
+
+using WTF::memcpySpan;
+using WTF::memsetSpan;
+

Source/WebCore/platform/audio/AudioArray.h

@@ -29,6 +29,7 @@
 #ifndef AudioArray_h
 #define AudioArray_h
 
+#include <span>
 #include <string.h>
 #include <wtf/CheckedArithmetic.h>
 #include <wtf/FastMalloc.h>
@@ -72,6 +73,8 @@ class AudioArray {
         zero();
     }
 
+    std::span<T> span() { return { data(), size() }; }
+    std::span<const T> span() const { return { data(), size() }; }
     T* data() { return m_allocation; }
     const T* data() const { return m_allocation; }
     size_t size() const { return m_size; }

Source/WebCore/platform/audio/AudioBus.cpp

@@ -544,19 +544,18 @@ RefPtr<AudioBus> AudioBus::createBySampleRateConverting(const AudioBus* sourceBu
     }
 
     // Calculate destination length based on the sample-rates.
-    int sourceLength = resamplerSourceBus->length();
-    int destinationLength = sourceLength / sampleRateRatio;
+    size_t sourceLength = resamplerSourceBus->length();
+    size_t destinationLength = sourceLength / sampleRateRatio;
 
     // Create destination bus with same number of channels.
     unsigned numberOfDestinationChannels = resamplerSourceBus->numberOfChannels();
     RefPtr<AudioBus> destinationBus = create(numberOfDestinationChannels, destinationLength);
 
     // Sample-rate convert each channel.
     for (unsigned i = 0; i < numberOfDestinationChannels; ++i) {
-        const float* source = resamplerSourceBus->channel(i)->data();
-        float* destination = destinationBus->channel(i)->mutableData();
-
-        SincResampler::processBuffer(source, destination, sourceLength, sampleRateRatio);
+        auto* sourceChannel = resamplerSourceBus->channel(i);
+        auto* destinationChannel = destinationBus->channel(i);
+        SincResampler::processBuffer(sourceChannel->span(), destinationChannel->mutableSpan(), sampleRateRatio);
     }
 
     destinationBus->clearSilentFlag();

Source/WebCore/platform/audio/AudioChannel.h

@@ -31,6 +31,7 @@
 
 #include "AudioArray.h"
 #include <memory>
+#include <span>
 #include <wtf/Noncopyable.h>
 
 namespace WebCore {
@@ -81,6 +82,9 @@ class AudioChannel final {
         m_length = newLength;
     }
 
+    std::span<const float> span() const { return { data(), length() }; }
+    std::span<float> mutableSpan() { return { mutableData(), length() }; }
+
     // Direct access to PCM sample data. Non-const accessor clears silent flag.
     float* mutableData()
     {

Source/WebCore/platform/audio/MultiChannelResampler.cpp

@@ -35,6 +35,7 @@
 #include "AudioBus.h"
 #include "SincResampler.h"
 #include <functional>
+#include <wtf/Algorithms.h>
 
 namespace WebCore {
 
@@ -66,7 +67,7 @@ void MultiChannelResampler::process(AudioBus* destination, size_t framesToProces
     ASSERT(m_numberOfChannels == destination->numberOfChannels());
     if (destination->numberOfChannels() == 1) {
         // Fast path when the bus is mono to avoid the chunking below.
-        m_kernels[0]->process(destination->channel(0)->mutableData(), framesToProcess);
+        m_kernels[0]->process(destination->channel(0)->mutableSpan(), framesToProcess);
         return;
     }
 
@@ -80,28 +81,30 @@ void MultiChannelResampler::process(AudioBus* destination, size_t framesToProces
 
         for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) {
             ASSERT(chunkSize == m_kernels[channelIndex]->chunkSize());
-            m_kernels[channelIndex]->process(destination->channel(channelIndex)->mutableData() + m_outputFramesReady, framesThisTime);
+            auto* channel = destination->channel(channelIndex);
+            m_kernels[channelIndex]->process(channel->mutableSpan().subspan(m_outputFramesReady), framesThisTime);
         }
 
         m_outputFramesReady += framesThisTime;
     }
 }
 
-void MultiChannelResampler::provideInputForChannel(float* buffer, size_t framesToProcess, unsigned channelIndex)
+void MultiChannelResampler::provideInputForChannel(std::span<float> buffer, size_t framesToProcess, unsigned channelIndex)
 {
     ASSERT(channelIndex < m_multiChannelBus->numberOfChannels());
     ASSERT(framesToProcess == m_multiChannelBus->length());
 
     if (!channelIndex) {
         // As an optimization, we use the provided buffer as memory for the first channel in the AudioBus. This avoids
         // having to memcpy() for the first channel.
-        m_multiChannelBus->setChannelMemory(0, buffer, framesToProcess);
+        RELEASE_ASSERT(framesToProcess <= buffer.size());
+        m_multiChannelBus->setChannelMemory(0, buffer.data(), framesToProcess);
         m_provideInput(m_multiChannelBus.get(), framesToProcess);
         return;
     }
 
     // Copy the channel data from what we received from m_multiChannelProvider.
-    memcpy(buffer, m_multiChannelBus->channel(channelIndex)->data(), sizeof(float) * framesToProcess);
+    memcpySpan(buffer.subspan(0, framesToProcess), m_multiChannelBus->channel(channelIndex)->span().subspan(0, framesToProcess));
 }
 
 } // namespace WebCore

Source/WebCore/platform/audio/MultiChannelResampler.h

@@ -49,7 +49,7 @@ class MultiChannelResampler final {
     void process(AudioBus* destination, size_t framesToProcess);
 
 private:
-    void provideInputForChannel(float* buffer, size_t framesToProcess, unsigned channelIndex);
+    void provideInputForChannel(std::span<float> buffer, size_t framesToProcess, unsigned channelIndex);
 
     // FIXME: the mac port can have a more highly optimized implementation based on CoreAudio
     // instead of SincResampler. For now the default implementation will be used on all ports.

Source/WebCore/platform/audio/SincResampler.cpp

@@ -34,6 +34,7 @@
 
 #include "AudioBus.h"
 #include "AudioUtilities.h"
+#include <wtf/Algorithms.h>
 #include <wtf/MathExtras.h>
 
 #if USE(ACCELERATE)
@@ -125,14 +126,14 @@ static size_t calculateChunkSize(unsigned blockSize, double scaleFactor)
     return blockSize / scaleFactor;
 }
 
-SincResampler::SincResampler(double scaleFactor, unsigned requestFrames, Function<void(float* buffer, size_t framesToProcess)>&& provideInput)
+SincResampler::SincResampler(double scaleFactor, unsigned requestFrames, Function<void(std::span<float> buffer, size_t framesToProcess)>&& provideInput)
     : m_scaleFactor(scaleFactor)
     , m_kernelStorage(kernelStorageSize)
     , m_requestFrames(requestFrames)
     , m_provideInput(WTFMove(provideInput))
     , m_inputBuffer(m_requestFrames + kernelSize) // See input buffer layout above.
-    , m_r1(m_inputBuffer.data())
-    , m_r2(m_inputBuffer.data() + kernelSize / 2)
+    , m_r1(m_inputBuffer.data(), m_inputBuffer.size())
+    , m_r2(m_inputBuffer.span().subspan(kernelSize / 2))
 {
     ASSERT(m_provideInput);
     ASSERT(m_requestFrames > 0);
@@ -145,18 +146,18 @@ void SincResampler::updateRegions(bool isSecondLoad)
 {
     // Setup various region pointers in the buffer (see diagram above). If we're
     // on the second load we need to slide m_r0 to the right by kernelSize / 2.
-    m_r0 = m_inputBuffer.data() + (isSecondLoad ? kernelSize : kernelSize / 2);
-    m_r3 = m_r0 + m_requestFrames - kernelSize;
-    m_r4 = m_r0 + m_requestFrames - kernelSize / 2;
-    m_blockSize = m_r4 - m_r2;
+    m_r0 = m_inputBuffer.span().subspan(isSecondLoad ? kernelSize : kernelSize / 2);
+    m_r3 = m_r0.subspan(m_requestFrames - kernelSize);
+    m_r4 = m_r0.subspan(m_requestFrames - kernelSize / 2);
+    m_blockSize = std::distance(m_r2.begin(), m_r4.begin());
     m_chunkSize = calculateChunkSize(m_blockSize, m_scaleFactor);
 
     // m_r1 at the beginning of the buffer.
-    ASSERT(m_r1 == m_inputBuffer.data());
+    ASSERT(m_r1.data() == m_inputBuffer.data());
     // m_r1 left of m_r2, m_r4 left of m_r3 and size correct.
-    ASSERT((m_r2 - m_r1) == (m_r4 - m_r3));
+    ASSERT(std::distance(m_r1.begin(), m_r2.begin()) == std::distance(m_r3.begin(), m_r4.begin()));
     // m_r2 left of r3.
-    ASSERT(m_r2 <= m_r3);
+    ASSERT(m_r2.begin() <= m_r3.begin());
 }
 
 void SincResampler::initializeKernel()
@@ -200,34 +201,29 @@ void SincResampler::initializeKernel()
     }
 }
 
-void SincResampler::processBuffer(const float* source, float* destination, unsigned numberOfSourceFrames, double scaleFactor)
+void SincResampler::processBuffer(std::span<const float> source, std::span<float> destination, double scaleFactor)
 {
-    SincResampler resampler(scaleFactor, AudioUtilities::renderQuantumSize, [source, numberOfSourceFrames](float* buffer, size_t framesToProcess) mutable {
+    RELEASE_ASSERT(destination.size() == static_cast<size_t>(source.size() / scaleFactor));
+    SincResampler resampler(scaleFactor, AudioUtilities::renderQuantumSize, [&source](std::span<float> buffer, size_t framesToProcess) mutable {
         // Clamp to number of frames available and zero-pad.
-        size_t framesToCopy = std::min<size_t>(numberOfSourceFrames, framesToProcess);
-        memcpy(buffer, source, sizeof(float) * framesToCopy);
+        size_t framesToCopy = std::min(source.size(), framesToProcess);
+        memcpySpan(buffer.subspan(0, framesToCopy), source.subspan(0, framesToCopy));
 
         // Zero-pad if necessary.
         if (framesToCopy < framesToProcess)
-            memset(buffer + framesToCopy, 0, sizeof(float) * (framesToProcess - framesToCopy));
+            memsetSpan(buffer.subspan(framesToCopy, framesToProcess - framesToCopy), 0);
 
-        numberOfSourceFrames -= framesToCopy;
-        source += framesToCopy;
+        source = source.subspan(framesToCopy);
     });
 
-    unsigned numberOfDestinationFrames = static_cast<unsigned>(numberOfSourceFrames / scaleFactor);
-    unsigned remaining = numberOfDestinationFrames;
-
-    while (remaining) {
-        unsigned framesThisTime = std::min<unsigned>(remaining, AudioUtilities::renderQuantumSize);
+    while (!destination.empty()) {
+        unsigned framesThisTime = std::min<size_t>(destination.size(), AudioUtilities::renderQuantumSize);
         resampler.process(destination, framesThisTime);
-
-        destination += framesThisTime;
-        remaining -= framesThisTime;
+        destination = destination.subspan(framesThisTime);
     }
 }
 
-void SincResampler::process(float* destination, size_t framesToProcess)
+void SincResampler::process(std::span<float> destination, size_t framesToProcess)
 {
     unsigned numberOfDestinationFrames = framesToProcess;
 
@@ -240,6 +236,7 @@ void SincResampler::process(float* destination, size_t framesToProcess)
 
     // Step (2)
 
+    size_t destinationIndex = 0;
     while (numberOfDestinationFrames) {
         while (m_virtualSourceIndex < m_blockSize) {
             // m_virtualSourceIndex lies in between two kernel offsets so figure out what they are.
@@ -249,20 +246,20 @@ void SincResampler::process(float* destination, size_t framesToProcess)
             double virtualOffsetIndex = subsampleRemainder * numberOfKernelOffsets;
             int offsetIndex = static_cast<int>(virtualOffsetIndex);
 
-            float* k1 = m_kernelStorage.data() + offsetIndex * kernelSize;
-            float* k2 = k1 + kernelSize;
+            auto k1 = m_kernelStorage.span().subspan(offsetIndex * kernelSize);
+            auto k2 = k1.subspan(kernelSize);
 
             // Ensure |k1|, |k2| are 16-byte aligned for SIMD usage. Should always be true so long as kernelSize is a multiple of 16.
-            ASSERT(!(reinterpret_cast<uintptr_t>(k1) & 0x0F));
-            ASSERT(!(reinterpret_cast<uintptr_t>(k2) & 0x0F));
+            ASSERT(!(reinterpret_cast<uintptr_t>(k1.data()) & 0x0F));
+            ASSERT(!(reinterpret_cast<uintptr_t>(k2.data()) & 0x0F));
 
             // Initialize input pointer based on quantized m_virtualSourceIndex.
-            float* inputP = m_r1 + sourceIndexI;
+            auto inputP = m_r1.subspan(sourceIndexI);
 
             // Figure out how much to weight each kernel's "convolution".
             double kernelInterpolationFactor = virtualOffsetIndex - offsetIndex;
 
-            *destination++ = convolve(inputP, k1, k2, kernelInterpolationFactor);
+            destination[destinationIndex++] = convolve(inputP.data(), k1.data(), k2.data(), kernelInterpolationFactor);
 
             // Advance the virtual index.
             m_virtualSourceIndex += m_scaleFactor;
@@ -278,10 +275,10 @@ void SincResampler::process(float* destination, size_t framesToProcess)
 
         // Step (3) Copy r3 to r1.
         // This wraps the last input frames back to the start of the buffer.
-        memcpy(m_r1, m_r3, sizeof(float) * kernelSize);
+        memcpySpan(m_r1.subspan(0, kernelSize), m_r3.subspan(0, kernelSize));
 
         // Step (4) -- Reinitialize regions if necessary.
-        if (m_r0 == m_r2)
+        if (m_r0.data() == m_r2.data())
             updateRegions(true);
 
         // Step (5)

Source/WebCore/platform/audio/SincResampler.h

@@ -31,6 +31,7 @@
 
 #include "AudioArray.h"
 #include "AudioSourceProvider.h"
+#include <span>
 #include <wtf/Function.h>
 #include <wtf/RefPtr.h>
 
@@ -43,17 +44,17 @@ class SincResampler final {
 public:
     // scaleFactor == sourceSampleRate / destinationSampleRate
     // requestFrames controls the size in frames of the buffer requested by each provideInput() call.
-    SincResampler(double scaleFactor, unsigned requestFrames, Function<void(float* buffer, size_t framesToProcess)>&& provideInput);
+    SincResampler(double scaleFactor, unsigned requestFrames, Function<void(std::span<float> buffer, size_t framesToProcess)>&& provideInput);
 
     size_t chunkSize() const { return m_chunkSize; }
 
-    // Processes numberOfSourceFrames from source to produce numberOfSourceFrames / scaleFactor frames in destination.
-    WEBCORE_EXPORT static void processBuffer(const float* source, float* destination, unsigned numberOfSourceFrames, double scaleFactor);
+    // Processes samples in `source` to produce source.size() / scaleFactor frames in `destination`.
+    WEBCORE_EXPORT static void processBuffer(std::span<const float> source, std::span<float> destination, double scaleFactor);
 
     // Process with provideInput callback function for streaming applications.
-    void process(float* destination, size_t framesToProcess);
+    void process(std::span<float> destination, size_t framesToProcess);
 
-protected:
+private:
     void initializeKernel();
     void updateRegions(bool isSecondLoad);
 
@@ -72,23 +73,23 @@ class SincResampler final {
     // This is the number of destination frames we generate per processing pass on the buffer.
     unsigned m_requestFrames;
 
-    Function<void(float* buffer, size_t framesToProcess)> m_provideInput;
+    Function<void(std::span<float> buffer, size_t framesToProcess)> m_provideInput;
 
     // The number of source frames processed per pass.
-    unsigned m_blockSize { 0 };
+    size_t m_blockSize { 0 };
 
     size_t m_chunkSize { 0 };
 
     // Source is copied into this buffer for each processing pass.
     AudioFloatArray m_inputBuffer;
 
-    // Pointers to the various regions inside |m_inputBuffer|. See the diagram at
+    // Spans to the various regions inside |m_inputBuffer|. See the diagram at
     // the top of the .cpp file for more information.
-    float* m_r0 { nullptr };
-    float* const m_r1 { nullptr };
-    float* const m_r2 { nullptr };
-    float* m_r3 { nullptr };
-    float* m_r4 { nullptr };
+    std::span<float> m_r0;
+    const std::span<float> m_r1;
+    const std::span<float> m_r2;
+    std::span<float> m_r3;
+    std::span<float> m_r4;
 
     // The buffer is primed once at the very beginning of processing.
     bool m_isBufferPrimed { false };