From bcd222cfae209b6c4908f7145c14b7eb2900435d Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 21 Jun 2023 19:37:10 -0400 Subject: [PATCH] Ladybird+LibWeb+WebContent: Prevent out-of-bounds seeking audio elements It's currently possible to seek to the total sample count of an audio loader. We must limit seeking to one less than that count. This mistake was duplicated in both AudioCodecPluginSerenity/Ladybird, so the computation was moved to a helper in the base AudioCodecPlugin. --- Ladybird/AudioCodecPluginLadybird.cpp | 14 +++----------- .../Libraries/LibWeb/Platform/AudioCodecPlugin.cpp | 12 ++++++++++++ .../Libraries/LibWeb/Platform/AudioCodecPlugin.h | 1 + .../WebContent/AudioCodecPluginSerenity.cpp | 6 +----- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Ladybird/AudioCodecPluginLadybird.cpp b/Ladybird/AudioCodecPluginLadybird.cpp index eab2e9624e..6c58a932dd 100644 --- a/Ladybird/AudioCodecPluginLadybird.cpp +++ b/Ladybird/AudioCodecPluginLadybird.cpp @@ -160,22 +160,14 @@ private: paused = Paused::Yes; break; - case AudioTask::Type::Seek: { + case AudioTask::Type::Seek: VERIFY(task.data.has_value()); - auto position = *task.data; + m_position = Web::Platform::AudioCodecPlugin::set_loader_position(m_loader, *task.data, m_duration, audio_output->format().sampleRate()); - auto duration = static_cast(this->duration().to_milliseconds()) / 1000.0; - position = position / duration * static_cast(m_loader->total_samples()); - - m_loader->seek(static_cast(position)).release_value_but_fixme_should_propagate_errors(); - - if (paused == Paused::Yes) { - m_position = Web::Platform::AudioCodecPlugin::current_loader_position(m_loader, audio_output->format().sampleRate()); + if (paused == Paused::Yes) Q_EMIT playback_position_updated(m_position); - } break; - } case AudioTask::Type::Volume: VERIFY(task.data.has_value()); diff --git a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp index 8b8e19a681..c44e985f0d 100644 --- a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp +++ b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp @@ -40,6 +40,18 @@ ErrorOr> AudioCodecPlugin::read_samples_from_loader(Au return FixedArray::create(resampler.resample(buffer_or_error.release_value()).span()); } +Duration AudioCodecPlugin::set_loader_position(Audio::Loader& loader, double position, Duration duration, size_t device_sample_rate) +{ + if (loader.total_samples() == 0) + return current_loader_position(loader, device_sample_rate); + + auto duration_value = static_cast(duration.to_milliseconds()) / 1000.0; + position = position / duration_value * static_cast(loader.total_samples() - 1); + + loader.seek(static_cast(position)).release_value_but_fixme_should_propagate_errors(); + return current_loader_position(loader, device_sample_rate); +} + Duration AudioCodecPlugin::current_loader_position(Audio::Loader const& loader, size_t device_sample_rate) { auto samples_played = static_cast(loader.loaded_samples()); diff --git a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h index 1b0efb1fd3..6c15ca88d9 100644 --- a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h +++ b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h @@ -24,6 +24,7 @@ public: virtual ~AudioCodecPlugin(); static ErrorOr> read_samples_from_loader(Audio::Loader&, size_t samples_to_load, size_t device_sample_rate); + static Duration set_loader_position(Audio::Loader&, double position, Duration duration, size_t device_sample_rate); static Duration current_loader_position(Audio::Loader const&, size_t device_sample_rate); virtual void resume_playback() = 0; diff --git a/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp b/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp index 9934a355de..0f4d2d8023 100644 --- a/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp +++ b/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp @@ -90,11 +90,7 @@ void AudioCodecPluginSerenity::set_volume(double volume) void AudioCodecPluginSerenity::seek(double position) { - auto duration = static_cast(this->duration().to_milliseconds()) / 1000.0; - position = position / duration * static_cast(m_loader->total_samples()); - - m_loader->seek(static_cast(position)).release_value_but_fixme_should_propagate_errors(); - m_position = current_loader_position(m_loader, m_device_sample_rate); + m_position = set_loader_position(m_loader, position, m_duration, m_device_sample_rate); if (on_playback_position_updated) on_playback_position_updated(m_position);