diff --git a/Userland/Libraries/LibArchive/Tar.h b/Userland/Libraries/LibArchive/Tar.h index d3e9973e16..6b9ddd86a1 100644 --- a/Userland/Libraries/LibArchive/Tar.h +++ b/Userland/Libraries/LibArchive/Tar.h @@ -51,14 +51,15 @@ constexpr StringView posix1_tar_magic = ""sv; // POSIX.1-1988 format magic constexpr StringView posix1_tar_version = ""sv; // POSIX.1-1988 format version template -static size_t get_field_as_integral(char const (&field)[N]) +static ErrorOr get_field_as_integral(char const (&field)[N]) { size_t value = 0; for (size_t i = 0; i < N; ++i) { if (field[i] == 0 || field[i] == ' ') break; - VERIFY(field[i] >= '0' && field[i] <= '7'); + if (field[i] < '0' || field[i] > '7') + return Error::from_string_literal("Passed a non-octal value"); value *= 8; value += field[i] - '0'; } @@ -89,21 +90,21 @@ static void set_octal_field(char (&field)[N], TSource&& source) class [[gnu::packed]] TarFileHeader { public: StringView filename() const { return get_field_as_string_view(m_filename); } - mode_t mode() const { return get_field_as_integral(m_mode); } - uid_t uid() const { return get_field_as_integral(m_uid); } - gid_t gid() const { return get_field_as_integral(m_gid); } + ErrorOr mode() const { return TRY(get_field_as_integral(m_mode)); } + ErrorOr uid() const { return TRY(get_field_as_integral(m_uid)); } + ErrorOr gid() const { return TRY(get_field_as_integral(m_gid)); } // FIXME: support 2001-star size encoding - size_t size() const { return get_field_as_integral(m_size); } - time_t timestamp() const { return get_field_as_integral(m_timestamp); } - unsigned checksum() const { return get_field_as_integral(m_checksum); } + ErrorOr size() const { return TRY(get_field_as_integral(m_size)); } + ErrorOr timestamp() const { return TRY(get_field_as_integral(m_timestamp)); } + ErrorOr checksum() const { return TRY(get_field_as_integral(m_checksum)); } TarFileType type_flag() const { return TarFileType(m_type_flag); } StringView link_name() const { return { m_link_name, strlen(m_link_name) }; } StringView magic() const { return get_field_as_string_view(m_magic); } StringView version() const { return get_field_as_string_view(m_version); } StringView owner_name() const { return get_field_as_string_view(m_owner_name); } StringView group_name() const { return get_field_as_string_view(m_group_name); } - int major() const { return get_field_as_integral(m_major); } - int minor() const { return get_field_as_integral(m_minor); } + ErrorOr major() const { return TRY(get_field_as_integral(m_major)); } + ErrorOr minor() const { return TRY(get_field_as_integral(m_minor)); } // FIXME: support ustar filename prefix StringView prefix() const { return get_field_as_string_view(m_prefix); } diff --git a/Userland/Libraries/LibArchive/TarStream.cpp b/Userland/Libraries/LibArchive/TarStream.cpp index f7331e7dcf..d92ac1e374 100644 --- a/Userland/Libraries/LibArchive/TarStream.cpp +++ b/Userland/Libraries/LibArchive/TarStream.cpp @@ -25,7 +25,12 @@ size_t TarFileStream::read(Bytes bytes) if (has_any_error()) return 0; - auto to_read = min(bytes.size(), m_tar_stream.header().size() - m_tar_stream.m_file_offset); + auto header_size_or_error = m_tar_stream.header().size(); + if (header_size_or_error.is_error()) + return 0; + auto header_size = header_size_or_error.release_value(); + + 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; @@ -37,8 +42,13 @@ bool TarFileStream::unreliable_eof() const // verify that the stream has not advanced VERIFY(m_tar_stream.m_generation == m_generation); + auto header_size_or_error = m_tar_stream.header().size(); + if (header_size_or_error.is_error()) + return true; + auto header_size = header_size_or_error.release_value(); + return m_tar_stream.m_stream.unreliable_eof() - || m_tar_stream.m_file_offset >= m_tar_stream.header().size(); + || m_tar_stream.m_file_offset >= header_size; } bool TarFileStream::read_or_error(Bytes bytes) @@ -59,7 +69,12 @@ bool TarFileStream::discard_or_error(size_t count) // verify that the stream has not advanced VERIFY(m_tar_stream.m_generation == m_generation); - if (count > m_tar_stream.header().size() - m_tar_stream.m_file_offset) { + auto header_size_or_error = m_tar_stream.header().size(); + if (header_size_or_error.is_error()) + return false; + auto header_size = header_size_or_error.release_value(); + + if (count > header_size - m_tar_stream.m_file_offset) { return false; } m_tar_stream.m_file_offset += count; @@ -88,7 +103,13 @@ ErrorOr TarInputStream::advance() return Error::from_string_literal("Attempted to read a finished stream"); m_generation++; - VERIFY(m_stream.discard_or_error(block_ceiling(m_header.size()) - m_file_offset)); + + auto header_size_or_error = m_header.size(); + if (header_size_or_error.is_error()) + return header_size_or_error.release_error(); + auto header_size = header_size_or_error.release_value(); + + VERIFY(m_stream.discard_or_error(block_ceiling(header_size) - m_file_offset)); m_file_offset = 0; if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) { @@ -116,7 +137,10 @@ bool TarInputStream::valid() const return false; // POSIX.1-1988 tar does not have magic numbers, so we also need to verify the header checksum. - return header().checksum() == header().expected_checksum(); + if (header().checksum().is_error()) + return false; + + return header().checksum().release_value() == header().expected_checksum(); } TarFileStream TarInputStream::file_contents() diff --git a/Userland/Libraries/LibArchive/TarStream.h b/Userland/Libraries/LibArchive/TarStream.h index 37fed1e2fa..9f26208ae4 100644 --- a/Userland/Libraries/LibArchive/TarStream.h +++ b/Userland/Libraries/LibArchive/TarStream.h @@ -75,8 +75,13 @@ inline ErrorOr TarInputStream::for_each_extended_header(F func) Archive::TarFileStream file_stream = file_contents(); - ByteBuffer file_contents_buffer = TRY(ByteBuffer::create_zeroed(header().size())); - VERIFY(file_stream.read(file_contents_buffer) == header().size()); + auto header_size_or_error = header().size(); + if (header_size_or_error.is_error()) + return header_size_or_error.release_error(); + auto header_size = header_size_or_error.release_value(); + + ByteBuffer file_contents_buffer = TRY(ByteBuffer::create_zeroed(header_size)); + VERIFY(file_stream.read(file_contents_buffer) == header_size); StringView file_contents { file_contents_buffer }; diff --git a/Userland/Utilities/tar.cpp b/Userland/Utilities/tar.cpp index 0e952fb142..16c6852283 100644 --- a/Userland/Utilities/tar.cpp +++ b/Userland/Utilities/tar.cpp @@ -160,12 +160,17 @@ ErrorOr serenity_main(Main::Arguments arguments) String absolute_path = Core::File::absolute_path(filename); auto parent_path = LexicalPath(absolute_path).parent(); + auto header_mode_or_error = header.mode(); + if (header_mode_or_error.is_error()) + return header_mode_or_error.release_error(); + auto header_mode = header_mode_or_error.release_value(); + switch (header.type_flag()) { case Archive::TarFileType::NormalFile: case Archive::TarFileType::AlternateNormalFile: { MUST(Core::Directory::create(parent_path, Core::Directory::CreateDirectories::Yes)); - int fd = TRY(Core::System::open(absolute_path, O_CREAT | O_WRONLY, header.mode())); + int fd = TRY(Core::System::open(absolute_path, O_CREAT | O_WRONLY, header_mode)); Array buffer; size_t bytes_read; @@ -184,7 +189,7 @@ ErrorOr serenity_main(Main::Arguments arguments) case Archive::TarFileType::Directory: { MUST(Core::Directory::create(parent_path, Core::Directory::CreateDirectories::Yes)); - auto result_or_error = Core::System::mkdir(absolute_path, header.mode()); + auto result_or_error = Core::System::mkdir(absolute_path, header_mode); if (result_or_error.is_error() && result_or_error.error().code() != EEXIST) return result_or_error.error(); break;