diff --git a/Meta/Lagom/Fuzzers/FuzzTar.cpp b/Meta/Lagom/Fuzzers/FuzzTar.cpp index 82c54d9789..7637dbd36d 100644 --- a/Meta/Lagom/Fuzzers/FuzzTar.cpp +++ b/Meta/Lagom/Fuzzers/FuzzTar.cpp @@ -4,23 +4,35 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include +#include #include +#include +#include #include extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size) { - InputMemoryStream input_stream(ReadonlyBytes { data, size }); - Archive::TarInputStream tar_stream(input_stream); + // FIXME: Create a ReadonlyBytes variant of Core::Stream::MemoryStream. + auto input_stream_or_error = Core::Stream::MemoryStream::construct(Bytes { const_cast(data), size }); - if (!tar_stream.valid()) + if (input_stream_or_error.is_error()) return 0; - while (!tar_stream.finished()) { - auto const& header = tar_stream.header(); + auto tar_stream_or_error = Archive::TarInputStream::construct(input_stream_or_error.release_value()); + + if (tar_stream_or_error.is_error()) + return 0; + + auto tar_stream = tar_stream_or_error.release_value(); + + if (!tar_stream->valid()) + return 0; + + while (!tar_stream->finished()) { + auto const& header = tar_stream->header(); if (!header.content_is_like_extended_header()) { - if (tar_stream.advance().is_error()) + if (tar_stream->advance().is_error()) return 0; else continue; @@ -29,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size) switch (header.type_flag()) { case Archive::TarFileType::GlobalExtendedHeader: case Archive::TarFileType::ExtendedHeader: { - auto result = tar_stream.for_each_extended_header([&](StringView, StringView) {}); + auto result = tar_stream->for_each_extended_header([&](StringView, StringView) {}); if (result.is_error()) return 0; break; @@ -38,7 +50,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size) return 0; } - if (tar_stream.advance().is_error()) + if (tar_stream->advance().is_error()) return 0; } diff --git a/Userland/Libraries/LibArchive/TarStream.cpp b/Userland/Libraries/LibArchive/TarStream.cpp index adcfeb858a..adb489d7d1 100644 --- a/Userland/Libraries/LibArchive/TarStream.cpp +++ b/Userland/Libraries/LibArchive/TarStream.cpp @@ -26,10 +26,10 @@ ErrorOr TarFileStream::read(Bytes bytes) auto to_read = min(bytes.size(), header_size - m_tar_stream.m_file_offset); - auto nread = m_tar_stream.m_stream.read(bytes.trim(to_read)); - m_tar_stream.m_file_offset += nread; + auto slice = TRY(m_tar_stream.m_stream->read(bytes.trim(to_read))); + m_tar_stream.m_file_offset += slice.size(); - return bytes.slice(0, nread); + return slice; } bool TarFileStream::is_eof() const @@ -42,7 +42,7 @@ bool TarFileStream::is_eof() const return true; auto header_size = header_size_or_error.release_value(); - return m_tar_stream.m_stream.unreliable_eof() + return m_tar_stream.m_stream->is_eof() || m_tar_stream.m_file_offset >= header_size; } @@ -52,14 +52,24 @@ ErrorOr TarFileStream::write(ReadonlyBytes) VERIFY_NOT_REACHED(); } -TarInputStream::TarInputStream(InputStream& stream) - : m_stream(stream) +ErrorOr> TarInputStream::construct(NonnullOwnPtr stream) +{ + auto tar_stream = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TarInputStream(move(stream)))); + + // Try and read the header. + auto header_span = TRY(tar_stream->m_stream->read(Bytes(&tar_stream->m_header, sizeof(m_header)))); + if (header_span.size() != sizeof(m_header)) + return Error::from_string_literal("Failed to read the entire header"); + + // Discard the rest of the block. + TRY(tar_stream->m_stream->discard(block_size - sizeof(TarFileHeader))); + + return tar_stream; +} + +TarInputStream::TarInputStream(NonnullOwnPtr stream) + : m_stream(move(stream)) { - if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header))) || !m_stream.discard_or_error(block_size - sizeof(TarFileHeader))) { - m_finished = true; - m_stream.handle_any_error(); // clear out errors so we don't assert - return; - } } static constexpr unsigned long block_ceiling(unsigned long offset) @@ -69,26 +79,27 @@ static constexpr unsigned long block_ceiling(unsigned long offset) ErrorOr TarInputStream::advance() { - if (m_finished) - return Error::from_string_literal("Attempted to read a finished stream"); + if (finished()) + return Error::from_string_literal("Attempted to advance a finished stream"); m_generation++; - auto header_size = TRY(m_header.size()); - VERIFY(m_stream.discard_or_error(block_ceiling(header_size) - m_file_offset)); + // Discard the pending bytes of the current entry. + auto file_size = TRY(m_header.size()); + TRY(m_stream->discard(block_ceiling(file_size) - m_file_offset)); m_file_offset = 0; - if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) { - m_finished = true; - m_stream.handle_any_error(); // clear out errors so we don't assert - return Error::from_string_literal("Failed to read the header"); - } - if (!valid()) { - m_finished = true; - return {}; - } + // FIXME: This is not unlike the initial initialization. Maybe we should merge those two. + auto header_span = TRY(m_stream->read(Bytes(&m_header, sizeof(m_header)))); + if (header_span.size() != sizeof(m_header)) + return Error::from_string_literal("Failed to read the entire header"); + + if (!valid()) + return Error::from_string_literal("Header is not valid"); + + // Discard the rest of the header block. + TRY(m_stream->discard(block_size - sizeof(TarFileHeader))); - VERIFY(m_stream.discard_or_error(block_size - sizeof(TarFileHeader))); return {}; } @@ -111,7 +122,7 @@ bool TarInputStream::valid() const TarFileStream TarInputStream::file_contents() { - VERIFY(!m_finished); + VERIFY(!finished()); return TarFileStream(*this); } diff --git a/Userland/Libraries/LibArchive/TarStream.h b/Userland/Libraries/LibArchive/TarStream.h index a13ae8ac87..25c8359a81 100644 --- a/Userland/Libraries/LibArchive/TarStream.h +++ b/Userland/Libraries/LibArchive/TarStream.h @@ -33,9 +33,9 @@ private: class TarInputStream { public: - TarInputStream(InputStream&); + static ErrorOr> construct(NonnullOwnPtr); ErrorOr advance(); - bool finished() const { return m_finished; } + bool finished() const { return m_stream->is_eof(); } bool valid() const; TarFileHeader const& header() const { return m_header; } TarFileStream file_contents(); @@ -44,11 +44,12 @@ public: ErrorOr for_each_extended_header(F func); private: + TarInputStream(NonnullOwnPtr); + TarFileHeader m_header; - InputStream& m_stream; + NonnullOwnPtr m_stream; unsigned long m_file_offset { 0 }; int m_generation { 0 }; - bool m_finished { false }; friend class TarFileStream; }; diff --git a/Userland/Utilities/tar.cpp b/Userland/Utilities/tar.cpp index 8f157193e6..20be73318b 100644 --- a/Userland/Utilities/tar.cpp +++ b/Userland/Utilities/tar.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -61,22 +62,34 @@ ErrorOr serenity_main(Main::Arguments arguments) } if (list || extract) { - auto file = Core::File::standard_input(); - - if (!archive_file.is_empty()) - file = TRY(Core::File::open(archive_file, Core::OpenMode::ReadOnly)); - if (!directory.is_empty()) TRY(Core::System::chdir(directory)); - Core::InputFileStream file_stream(file); - Compress::GzipDecompressor gzip_stream(file_stream); + // FIXME: Remove these once we have smart pointers everywhere in LibArchive and LibCompress (or just ported the whole stack to Core::Stream). + // Until then, we have to hold on to _some_ instance of the file AK::Stream. + // Note that this is only in use together with gzip. + OwnPtr file_stream; - InputStream& file_input_stream = file_stream; - InputStream& gzip_input_stream = gzip_stream; - Archive::TarInputStream tar_stream((gzip) ? gzip_input_stream : file_input_stream); + auto input_stream = TRY([&]() -> ErrorOr> { + if (gzip) { + // FIXME: Port gzip to Core::Stream. + auto file = Core::File::standard_input(); + + if (!archive_file.is_empty()) + file = TRY(Core::File::open(archive_file, Core::OpenMode::ReadOnly)); + + file_stream = adopt_own(*new Core::InputFileStream(file)); + NonnullOwnPtr gzip_stream = make(*file_stream); + + return make(move(gzip_stream)); + } else { + return TRY(Core::Stream::File::open_file_or_standard_stream(archive_file, Core::Stream::OpenMode::Read)); + } + }()); + + auto tar_stream = TRY(Archive::TarInputStream::construct(move(input_stream))); // FIXME: implement ErrorOr? - if (!tar_stream.valid()) { + if (!tar_stream->valid()) { warnln("the provided file is not a well-formatted ustar file"); return 1; } @@ -98,14 +111,14 @@ ErrorOr serenity_main(Main::Arguments arguments) return {}; }; - while (!tar_stream.finished()) { - Archive::TarFileHeader const& header = tar_stream.header(); + while (!tar_stream->finished()) { + Archive::TarFileHeader const& header = tar_stream->header(); // Handle meta-entries earlier to avoid consuming the file content stream. if (header.content_is_like_extended_header()) { switch (header.type_flag()) { case Archive::TarFileType::GlobalExtendedHeader: { - TRY(tar_stream.for_each_extended_header([&](StringView key, StringView value) { + TRY(tar_stream->for_each_extended_header([&](StringView key, StringView value) { if (value.length() == 0) global_overrides.remove(key); else @@ -114,7 +127,7 @@ ErrorOr serenity_main(Main::Arguments arguments) break; } case Archive::TarFileType::ExtendedHeader: { - TRY(tar_stream.for_each_extended_header([&](StringView key, StringView value) { + TRY(tar_stream->for_each_extended_header([&](StringView key, StringView value) { local_overrides.set(key, value); })); break; @@ -124,11 +137,11 @@ ErrorOr serenity_main(Main::Arguments arguments) VERIFY_NOT_REACHED(); } - TRY(tar_stream.advance()); + TRY(tar_stream->advance()); continue; } - Archive::TarFileStream file_stream = tar_stream.file_contents(); + Archive::TarFileStream file_stream = tar_stream->file_contents(); // Handle other header types that don't just have an effect on extraction. switch (header.type_flag()) { @@ -143,7 +156,7 @@ ErrorOr serenity_main(Main::Arguments arguments) } local_overrides.set("path", long_name.to_string()); - TRY(tar_stream.advance()); + TRY(tar_stream->advance()); continue; } default: @@ -204,9 +217,8 @@ ErrorOr serenity_main(Main::Arguments arguments) // Non-global headers should be cleared after every file. local_overrides.clear(); - TRY(tar_stream.advance()); + TRY(tar_stream->advance()); } - file_stream.close(); return 0; }