From 845d403b8c175ff99793239404ca8657d95da104 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Sun, 1 Aug 2021 05:42:09 -0600 Subject: [PATCH] LibAudio: Handle stream errors in FlacLoader The FlacLoader already has numerous checks for invalid data reads and for invalid stream states, but it never actually handles the stream errors on the stream object. By handling them properly we can actually run FuzzFlacLoader for longer than a few seconds before it hits the first assertion :^). --- Userland/Libraries/LibAudio/FlacLoader.cpp | 31 +++++++++++++++++----- Userland/Libraries/LibAudio/FlacLoader.h | 26 +++++++++++++++--- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/Userland/Libraries/LibAudio/FlacLoader.cpp b/Userland/Libraries/LibAudio/FlacLoader.cpp index 08e32ba7ad..5f770dc8c0 100644 --- a/Userland/Libraries/LibAudio/FlacLoader.cpp +++ b/Userland/Libraries/LibAudio/FlacLoader.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -33,6 +34,8 @@ FlacLoaderPlugin::FlacLoaderPlugin(const StringView& path) m_stream = make(Core::InputFileStream(*m_file)); reset(); + if (!m_valid) + return; m_resampler = make>(m_sample_rate, 44100); } @@ -49,6 +52,8 @@ FlacLoaderPlugin::FlacLoaderPlugin(const ByteBuffer& buffer) if (!m_valid) return; reset(); + if (!m_valid) + return; m_resampler = make>(m_sample_rate, 44100); } @@ -70,10 +75,12 @@ bool FlacLoaderPlugin::parse_header() } return InputBitStream(m_stream->get()); }(); + ScopeGuard clear_bit_input_errors([&bit_input] { bit_input.handle_any_error(); }); #define CHECK_OK(msg) \ do { \ if (!ok) { \ + m_stream->handle_any_error(); \ m_error_string = String::formatted("Parsing failed: {}", msg); \ return {}; \ } \ @@ -94,6 +101,7 @@ bool FlacLoaderPlugin::parse_header() CHECK_OK("First block type"); InputMemoryStream streaminfo_data_memory(streaminfo.data.bytes()); InputBitStream streaminfo_data(streaminfo_data_memory); + ScopeGuard clear_streaminfo_errors([&streaminfo_data] { streaminfo_data.handle_any_error(); }); // STREAMINFO block m_min_block_size = streaminfo_data.read_bits_big_endian(16); @@ -125,9 +133,13 @@ bool FlacLoaderPlugin::parse_header() } m_total_samples = streaminfo_data.read_bits_big_endian(36); + ok = ok && (m_total_samples > 0); + CHECK_OK("Number of samples"); // Parse checksum into a buffer first ByteBuffer md5_checksum = ByteBuffer::create_uninitialized(128 / 8); - streaminfo_data.read(md5_checksum); + auto md5_bytes_read = streaminfo_data.read(md5_checksum); + ok = ok && (md5_bytes_read == md5_checksum.size()); + CHECK_OK("MD5 Checksum"); md5_checksum.bytes().copy_to({ m_md5_checksum, sizeof(m_md5_checksum) }); // Parse other blocks @@ -143,6 +155,11 @@ bool FlacLoaderPlugin::parse_header() CHECK_OK(m_error_string); } + if (m_stream->handle_any_error()) { + m_error_string = "Parsing failed: Stream"; + return false; + } + if constexpr (AFLACLOADER_DEBUG) { // HACK: u128 should be able to format itself StringBuilder checksum_string; @@ -203,11 +220,13 @@ void FlacLoaderPlugin::reset() void FlacLoaderPlugin::seek(const int position) { - m_stream->seek(position); + if (!m_stream->seek(position)) { + m_error_string = String::formatted("Invalid seek position {}", position); + m_valid = false; + } } -// TODO implement these -RefPtr FlacLoaderPlugin::get_more_samples([[maybe_unused]] size_t max_bytes_to_read_from_input) +RefPtr FlacLoaderPlugin::get_more_samples(size_t max_bytes_to_read_from_input) { Vector samples; ssize_t remaining_samples = m_total_samples - m_loaded_samples; @@ -220,7 +239,7 @@ RefPtr FlacLoaderPlugin::get_more_samples([[maybe_unused]] size_t max_by if (!m_current_frame.has_value()) { next_frame(); if (!m_error_string.is_empty()) { - dbgln("Frame parsing error: {}", m_error_string); + m_error_string = String::formatted("Frame parsing error: {}", m_error_string); return nullptr; } // HACK: Test the start of the next subframe @@ -248,6 +267,7 @@ void FlacLoaderPlugin::next_frame() if (!ok) { \ m_error_string = String::formatted("Frame parsing failed: {}", msg); \ bit_stream.align_to_byte_boundary(); \ + bit_stream.handle_any_error(); \ dbgln_if(AFLACLOADER_DEBUG, "Crash in FLAC loader: next bytes are {:x}", bit_stream.read_bits_big_endian(32)); \ return; \ } \ @@ -834,5 +854,4 @@ i32 rice_to_signed(u32 x) // copies the sign's sign onto the actual magnitude of x return (i32)(sign ^ (x >> 1)); } - } diff --git a/Userland/Libraries/LibAudio/FlacLoader.h b/Userland/Libraries/LibAudio/FlacLoader.h index 0b75ff5dfd..1542ae3911 100644 --- a/Userland/Libraries/LibAudio/FlacLoader.h +++ b/Userland/Libraries/LibAudio/FlacLoader.h @@ -22,11 +22,26 @@ class FlacInputStream : public Variant public: using Variant::Variant; - void seek(size_t pos) + bool seek(size_t pos) { - this->visit( - [&](auto& stream) { + return this->visit( + [&](Core::InputFileStream& stream) { + return stream.seek(pos); + }, + [&](InputMemoryStream& stream) { + if (pos >= stream.bytes().size()) { + return false; + } stream.seek(pos); + return true; + }); + } + + bool handle_any_error() + { + return this->visit( + [&](auto& stream) { + return stream.handle_any_error(); }); } @@ -56,6 +71,11 @@ class FlacLoaderPlugin : public LoaderPlugin { public: FlacLoaderPlugin(const StringView& path); FlacLoaderPlugin(const ByteBuffer& buffer); + ~FlacLoaderPlugin() + { + if (m_stream) + m_stream->handle_any_error(); + } virtual bool sniff() override;