From 484c0edafc0d4d004243b23b1d833e836d3e449e Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Wed, 16 Feb 2022 13:33:15 -0700 Subject: [PATCH] LibArchive: Refactor zip header handling The directory headers have some common code for reading. --- Userland/Libraries/LibArchive/Zip.cpp | 8 ++-- Userland/Libraries/LibArchive/Zip.h | 66 +++++++++++++++------------ 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/Userland/Libraries/LibArchive/Zip.cpp b/Userland/Libraries/LibArchive/Zip.cpp index 7bc16c48a4d..8347e8c97d2 100644 --- a/Userland/Libraries/LibArchive/Zip.cpp +++ b/Userland/Libraries/LibArchive/Zip.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2022, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -15,8 +16,9 @@ bool Zip::find_end_of_central_directory_offset(ReadonlyBytes buffer, size_t& off if (buffer.size() < (sizeof(EndOfCentralDirectory) - sizeof(u8*)) + backwards_offset) return false; - auto signature_offset = (buffer.size() - (sizeof(EndOfCentralDirectory) - sizeof(u8*)) - backwards_offset); - if (memcmp(buffer.data() + signature_offset, end_of_central_directory_signature, sizeof(end_of_central_directory_signature)) == 0) { + auto const signature_offset = (buffer.size() - (sizeof(EndOfCentralDirectory) - sizeof(u8*)) - backwards_offset); + if (auto signature = ReadonlyBytes { buffer.data() + signature_offset, EndOfCentralDirectory::signature.size() }; + signature == EndOfCentralDirectory::signature) { offset = signature_offset; return true; } @@ -156,7 +158,7 @@ void ZipOutputStream::finish() central_directory_record.internal_attributes = 0; central_directory_record.external_attributes = member.is_directory ? zip_directory_external_attribute : 0; central_directory_record.local_file_header_offset = file_header_offset; // FIXME: we assume the wrapped output stream was never written to before us - file_header_offset += sizeof(local_file_header_signature) + (sizeof(LocalFileHeader) - (sizeof(u8*) * 3)) + member.name.length() + member.compressed_data.size(); + file_header_offset += sizeof(LocalFileHeader::signature) + (sizeof(LocalFileHeader) - (sizeof(u8*) * 3)) + member.name.length() + member.compressed_data.size(); central_directory_record.name = (const u8*)(member.name.characters()); central_directory_record.extra_data = nullptr; central_directory_record.comment = nullptr; diff --git a/Userland/Libraries/LibArchive/Zip.h b/Userland/Libraries/LibArchive/Zip.h index 8eee39d9c0d..bca2b4fd750 100644 --- a/Userland/Libraries/LibArchive/Zip.h +++ b/Userland/Libraries/LibArchive/Zip.h @@ -1,14 +1,15 @@ /* * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2022, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once +#include #include #include -#include #include #include #include @@ -16,10 +17,24 @@ namespace Archive { +template +static bool read_helper(ReadonlyBytes buffer, T* self) +{ + if (buffer.size() < T::signature.size() + fields_size) + return false; + if (buffer.slice(0, T::signature.size()) != T::signature) + return false; + memcpy(self, buffer.data() + T::signature.size(), fields_size); + return true; +} + // NOTE: Due to the format of zip files compression is streamed and decompression is random access. -static constexpr u8 end_of_central_directory_signature[] = { 0x50, 0x4b, 0x05, 0x06 }; // 'PK\x05\x06' +static constexpr auto signature_length = 4; + struct [[gnu::packed]] EndOfCentralDirectory { + static constexpr Array signature = { 0x50, 0x4b, 0x05, 0x06 }; // 'PK\x05\x06' + u16 disk_number; u16 central_directory_start_disk; u16 disk_records_count; @@ -31,21 +46,18 @@ struct [[gnu::packed]] EndOfCentralDirectory { bool read(ReadonlyBytes buffer) { - auto fields_size = sizeof(EndOfCentralDirectory) - sizeof(u8*); - if (buffer.size() < sizeof(end_of_central_directory_signature) + fields_size) + constexpr auto fields_size = sizeof(EndOfCentralDirectory) - (sizeof(u8*) * 1); + if (!read_helper(buffer, this)) return false; - if (memcmp(buffer.data(), end_of_central_directory_signature, sizeof(end_of_central_directory_signature)) != 0) + if (buffer.size() < signature.size() + fields_size + comment_length) return false; - memcpy(reinterpret_cast(&disk_number), buffer.data() + sizeof(end_of_central_directory_signature), fields_size); - if (buffer.size() < sizeof(end_of_central_directory_signature) + fields_size + comment_length) - return false; - comment = buffer.data() + sizeof(end_of_central_directory_signature) + fields_size; + comment = buffer.data() + signature.size() + fields_size; return true; } void write(OutputStream& stream) const { - stream.write_or_error({ end_of_central_directory_signature, sizeof(end_of_central_directory_signature) }); + stream.write_or_error(signature); stream << disk_number; stream << central_directory_start_disk; stream << disk_records_count; @@ -58,8 +70,9 @@ struct [[gnu::packed]] EndOfCentralDirectory { } }; -static constexpr u8 central_directory_record_signature[] = { 0x50, 0x4b, 0x01, 0x02 }; // 'PK\x01\x02' struct [[gnu::packed]] CentralDirectoryRecord { + static constexpr Array signature = { 0x50, 0x4b, 0x01, 0x02 }; // 'PK\x01\x02' + u16 made_by_version; u16 minimum_version; u16 general_purpose_flags; @@ -82,15 +95,12 @@ struct [[gnu::packed]] CentralDirectoryRecord { bool read(ReadonlyBytes buffer) { - auto fields_size = sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3); - if (buffer.size() < sizeof(central_directory_record_signature) + fields_size) + constexpr auto fields_size = sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3); + if (!read_helper(buffer, this)) return false; - if (memcmp(buffer.data(), central_directory_record_signature, sizeof(central_directory_record_signature)) != 0) + if (buffer.size() < size()) return false; - memcpy(reinterpret_cast(&made_by_version), buffer.data() + sizeof(central_directory_record_signature), fields_size); - if (buffer.size() < sizeof(end_of_central_directory_signature) + fields_size + comment_length + name_length + extra_data_length) - return false; - name = buffer.data() + sizeof(central_directory_record_signature) + fields_size; + name = buffer.data() + signature.size() + fields_size; extra_data = name + name_length; comment = extra_data + extra_data_length; return true; @@ -98,7 +108,7 @@ struct [[gnu::packed]] CentralDirectoryRecord { void write(OutputStream& stream) const { - stream.write_or_error({ central_directory_record_signature, sizeof(central_directory_record_signature) }); + stream.write_or_error(signature); stream << made_by_version; stream << minimum_version; stream << general_purpose_flags; @@ -125,13 +135,14 @@ struct [[gnu::packed]] CentralDirectoryRecord { [[nodiscard]] size_t size() const { - return sizeof(central_directory_record_signature) + (sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3)) + name_length + extra_data_length + comment_length; + return signature.size() + (sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3)) + name_length + extra_data_length + comment_length; } }; static constexpr u32 zip_directory_external_attribute = 1 << 4; -static constexpr u8 local_file_header_signature[] = { 0x50, 0x4b, 0x03, 0x04 }; // 'PK\x03\x04' struct [[gnu::packed]] LocalFileHeader { + static constexpr Array signature = { 0x50, 0x4b, 0x03, 0x04 }; // 'PK\x03\x04' + u16 minimum_version; u16 general_purpose_flags; u16 compression_method; @@ -148,15 +159,12 @@ struct [[gnu::packed]] LocalFileHeader { bool read(ReadonlyBytes buffer) { - auto fields_size = sizeof(LocalFileHeader) - (sizeof(u8*) * 3); - if (buffer.size() < sizeof(local_file_header_signature) + fields_size) + constexpr auto fields_size = sizeof(LocalFileHeader) - (sizeof(u8*) * 3); + if (!read_helper(buffer, this)) return false; - if (memcmp(buffer.data(), local_file_header_signature, sizeof(local_file_header_signature)) != 0) + if (buffer.size() < signature.size() + fields_size + name_length + extra_data_length + compressed_size) return false; - memcpy(reinterpret_cast(&minimum_version), buffer.data() + sizeof(local_file_header_signature), fields_size); - if (buffer.size() < sizeof(end_of_central_directory_signature) + fields_size + name_length + extra_data_length + compressed_size) - return false; - name = buffer.data() + sizeof(local_file_header_signature) + fields_size; + name = buffer.data() + signature.size() + fields_size; extra_data = name + name_length; compressed_data = extra_data + extra_data_length; return true; @@ -164,7 +172,7 @@ struct [[gnu::packed]] LocalFileHeader { void write(OutputStream& stream) const { - stream.write_or_error({ local_file_header_signature, sizeof(local_file_header_signature) }); + stream.write_or_error(signature); stream << minimum_version; stream << general_purpose_flags; stream << compression_method;