From 4c343c5f26a195ae1bb34646d403281472260c7f Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Mon, 16 Nov 2020 20:54:49 +0330 Subject: [PATCH] AK: Fix OOB access in DuplexMemoryStream::offset_of() This fixes an OOB access when the last read/written chunk is empty (as we _just_ started on a new chunk). Also adds a test case to TestMemoryStream. Found via human fuzzing in the shell: ```sh for $(cat /dev/urandom) { clear match $it { ?* as (x) { echo $x sleep 1 } } } ``` would assert at some point. --- AK/MemoryStream.h | 10 +++++++--- AK/Tests/TestMemoryStream.cpp | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/AK/MemoryStream.h b/AK/MemoryStream.h index 90bab32c428..26618637eb4 100644 --- a/AK/MemoryStream.h +++ b/AK/MemoryStream.h @@ -221,22 +221,26 @@ public: return true; } + // FIXME: Does not read across chunk boundaries + // Perhaps implement AK::memmem() for iterators? Optional offset_of(ReadonlyBytes value) const { if (value.size() > size()) return {}; // First, find which chunk we're in. - auto chunk_index = (m_read_offset - m_base_offset) / chunk_size; + auto chunk_index = min((m_read_offset - m_base_offset) / chunk_size, m_chunks.size() - 1); auto last_written_chunk_index = (m_write_offset - m_base_offset) / chunk_size; auto first_chunk_index = chunk_index; auto last_written_chunk_offset = m_write_offset % chunk_size; auto first_chunk_offset = m_read_offset % chunk_size; size_t last_chunk_offset = 0; auto found_value = false; + auto chunk_index_max_bound = last_written_chunk_offset > 0 ? last_written_chunk_index + 1 : last_written_chunk_index; - for (; chunk_index <= last_written_chunk_index; ++chunk_index) { - auto chunk_bytes = m_chunks[chunk_index].bytes(); + for (; chunk_index < chunk_index_max_bound; ++chunk_index) { + auto& chunk = m_chunks[chunk_index]; + auto chunk_bytes = chunk.bytes(); size_t chunk_offset = 0; if (chunk_index == last_written_chunk_index) { chunk_bytes = chunk_bytes.slice(0, last_written_chunk_offset); diff --git a/AK/Tests/TestMemoryStream.cpp b/AK/Tests/TestMemoryStream.cpp index 789e9c488e5..766c1690c8a 100644 --- a/AK/Tests/TestMemoryStream.cpp +++ b/AK/Tests/TestMemoryStream.cpp @@ -194,4 +194,18 @@ TEST_CASE(new_output_memory_stream) EXPECT_EQ(stream.bytes().size(), 2u); } +TEST_CASE(offset_of_out_of_bounds) +{ + Array target { 0xff, 0xff, 0xff, 0xff }; + + Array whole_chunk; + whole_chunk.span().fill(0); + + DuplexMemoryStream stream; + + stream << whole_chunk; + + EXPECT(!stream.offset_of(target).has_value()); +} + TEST_MAIN(MemoryStream)