Use libgtest_prod_headers. am: 03abb6e4e1 am: 7914b08513 am: 469fd84380 Original change: https://android-review.googlesource.com/c/platform/system/libziparchive/+/1682059 Change-Id: I448a17be0704a5f537984335e341a993d2190196
diff --git a/Android.bp b/Android.bp index 7ed9ba5..8226d39 100644 --- a/Android.bp +++ b/Android.bp
@@ -19,6 +19,7 @@ cc_defaults { name: "libziparchive_flags", + cpp_std: "c++2a", cflags: [ // ZLIB_CONST turns on const for input buffers, which is pretty standard. "-DZLIB_CONST", @@ -61,6 +62,7 @@ cc_defaults { name: "libziparchive_defaults", + local_include_dirs: ["incfs_support/include/"], srcs: [ "zip_archive.cc", "zip_archive_stream_entry.cc", @@ -89,8 +91,29 @@ export_include_dirs: ["include"], } -cc_library { - name: "libziparchive", +cc_defaults { + name: "incfs_support_defaults", + cflags: ["-DZIPARCHIVE_DISABLE_CALLBACK_API=1"], + export_include_dirs: ["incfs_support/include/"], + tidy: true, + tidy_checks: [ + "android-*", + "cert-*", + "clang-analyzer-security*", + "-cert-err34-c", + "clang-analyzer-security*", + // Disabling due to many unavoidable warnings from POSIX API usage. + "-google-runtime-int", + "-google-explicit-constructor", + // do not call 'longjmp'; consider using exception handling instead - library relies on it + "-cert-err52-cpp", + // typedef pointer used with const: all Zip* typedefs generate this when declared as const + "-misc-misplaced-const", + ], +} + +cc_defaults { + name: "libziparchive_lib_defaults", host_supported: true, vendor_available: true, product_available: true, @@ -127,6 +150,23 @@ min_sdk_version: "apex_inherit", } +cc_library { + name: "libziparchive", + defaults: ["libziparchive_lib_defaults"], + cflags: [ + // Disable incfs hardending code for the default library + "-DINCFS_SUPPORT_DISABLED=1", + ], +} + +cc_library_static { + name: "libziparchive_for_incfs", + defaults: ["libziparchive_lib_defaults", "incfs_support_defaults"], + srcs: [ + "incfs_support/signal_handling.cpp", + ], +} + // Tests. cc_test { name: "ziparchive-tests", @@ -211,7 +251,25 @@ cc_fuzz { name: "libziparchive_fuzzer", srcs: ["libziparchive_fuzzer.cpp"], - static_libs: ["libziparchive", "libbase", "libz", "liblog"], + static_libs: [ + "libziparchive", + "libbase", + "libz", + "liblog" + ], + host_supported: true, + corpus: ["testdata/*"], +} + +cc_fuzz { + name: "libziparchive_for_incfs_fuzzer", + srcs: ["libziparchive_fuzzer.cpp"], + static_libs: [ + "libziparchive_for_incfs", + "libbase", + "libz", + "liblog" + ], host_supported: true, corpus: ["testdata/*"], }
diff --git a/incfs_support/include/incfs_support/access.h b/incfs_support/include/incfs_support/access.h new file mode 100644 index 0000000..555371e --- /dev/null +++ b/incfs_support/include/incfs_support/access.h
@@ -0,0 +1,82 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <incfs_support/signal_handling.h> + +#include <optional> +#include <type_traits> + +namespace incfs { + +// Couple helper templates for access() implementation +template <class Arg, class Func> +using func_result = + std::remove_cvref_t<decltype(std::declval<Func>()(std::declval<Arg>()))>; + +template <class Arg, class Func> +constexpr auto is_void_func = std::is_same_v<func_result<Arg, Func>, void>; + +template <class Arg, class Func> +using optional_result = + std::conditional_t<is_void_func<Arg, Func>, bool, + std::optional<func_result<Arg, Func>>>; + +// +// A safer alternative for the SCOPED_SIGBUS_HANDLER() macro. +// +// This function accepts a closure, which is a block of code that may generate +// an error while accessing mapped memory. By using access() function you limit +// the potential for leaking resources to only the code in the closure. Anything +// allocated before, or after the call, works exactly as expected by the C++ +// standards, and will get its destructors run even in case of access error. But +// the code inside the closure still has the same issues as with the macros - no +// cleanup happens if reading memory fails. That's why it's better to not create +// any new objects inside of it, and not call external code, especially the one +// you don't control. +// +// While it's a safer alternative, one may still need to use the functions from +// incfs_support/util.h to ensure a proper memory cleanup. +// +// Code generated from the access() call is very similar to the raw macro, but +// sometimes the optimizer decides to make accessor() a real function call +// instead of inlining it. This is the only piece of overhead you may get, and +// that's why best practice is to start with this function first. Only fall back +// to SCOPED_SIGBUS_HANDLER() macro if it gives MEASURABLE improvement. +// +// @param ptr - mapped memory pointer to access +// @param accessor - a closure that runs under failure protection. Its return +// value wrapped into +// std::optional is the whole function return value +// @return - std::optional(access(ptr)) if no error happened, or +// std::nullopt on error; +// for a void-returning accessor a bool indicating if access +// was successful. + +template <class Ptr, class F> +auto access(Ptr ptr, F&& accessor) -> optional_result<Ptr, F> { + SCOPED_SIGBUS_HANDLER({ return {}; }); + + if constexpr (is_void_func<Ptr, F>) { + accessor(ptr); + return true; + } else { + return accessor(ptr); + } +} + +} // namespace incfs \ No newline at end of file
diff --git a/incfs_support/include/incfs_support/signal_handling.h b/incfs_support/include/incfs_support/signal_handling.h new file mode 100644 index 0000000..73e65ba --- /dev/null +++ b/incfs_support/include/incfs_support/signal_handling.h
@@ -0,0 +1,197 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +// +// This file defines low-level primitives for handling SIGBUS signals that get +// generated when accessing memory mapped files on IncFS and hitting a missing +// page. These primitives provide a way for a process to not crash, but require +// careful attention to use properly. +// +// A slightly safer alternative is in "access.h", you probably want it unless +// the performance is so critical that a single function call overhead is +// unacceptable. +// +// Macros: +// SCOPED_SIGBUS_HANDLER(code) +// SCOPED_SIGBUS_HANDLER_CONDITIONAL(condition, code) +// +// Both macros set up a thread-local handler for SIGBUS, and make the (code) +// passed as a parameter run when it happens; _CONDITIONAL version only sets the +// handler up if the (condition) is true, making it easier to write generic +// code. (code) runs in a context of a current function. Macros work via a +// setjmp/longjmp machinery, and those result in the following pitfalls: +// +// - Last action of the (code) MUST BE a return from the function. Current +// state is too badly corrupted to continue execution. +// -- Macro checks for code that failed to return and kills the process. +// - C++ destructors of the objects created since the macro DO NOT RUN. (code) +// has to manually clean up all of those. +// -- this means ALL allocated memory, locked mutexes, created temp files, +// etc. +// -- what this really means is it's better to not do anything that needs +// cleanup in the code protected with the macro +// - Signal handler jumps out of any level of a nested function call straight +// into (code); stack unwinding doesn't happen in a regular way - only the SP +// gets reset. Nested functions have even stronger restrictions: no destructor +// will run in them. +// -- under no circumstance one may call a user-supplied callback in a code +// protected with these macros. It's a recipe for a huge disaster. +// - If some other code overrides the signal handler, protections cease to +// work. Unfortunately, this is just the way signal handling works in Linux. +// +// Usage: +// +// In a function that will access mapped memory, as close to the access point as +// possible: +// +// int foo(char* mapped, long size) { +// ... +// SCOPED_SIGBUS_HANDLER(return -1); +// for (...) { +// <access |mapped|> +// } +// return 0; +// } +// +// If you need to perform some non-trivial cleanup: +// +// ... +// int fd = -1; +// SCOPED_SIGBUS_HANDLER({ +// if (fd >= 0) close(fd); +// return -1; +// }); +// ... +// <access |mapped|> +// fd = open(...); +// <access |mapped|> +// ... +// +// Cleanup: +// Pay attention when releasing the allocated resources - some functions may appear +// to do that while in reality they aren't; use the function(s) from incfs_support/util.h to +// do it safely, e.g.: +// +// std::vector<int> numbers; +// SCOPED_SIGBUS_HANDLER({ +// incfs::util::clearAndFree(numbers); +// // putting 'numbers.clear()' here instead would leak it as vector doesn't free its +// // capacity on that call, and SIGBUS may prevent its destructor from running. +// return -1; +// }); + +// Performance: each macro sets up a couple of thread-local variables and calls +// a single syscall, +// setjmp(). The only serious consideration is to not call it on each +// iteration of a tight loop, as syscalls aren't particularly instant. See +// "incfs-support-benchmark" project for more details. +// + +#include <sys/types.h> + +#if !defined(__BIONIC__) || INCFS_SUPPORT_DISABLED + +// IncFS signal handling isn't needed anywhere but on Android as of now; +// or if we're told it's not desired. +#define SCOPED_SIGBUS_HANDLER(code) +#define SCOPED_SIGBUS_HANDLER_CONDITIONAL(condition, code) + +#else + +#ifndef LOG_TAG +#define LOG_TAG "incfs:hardening" +#endif + +#include <log/log.h> +#include <setjmp.h> +#include <signal.h> +#include <string.h> + +namespace incfs { + +struct JmpBufState final { + jmp_buf buf; + bool armed = false; + + JmpBufState() = default; + JmpBufState(const JmpBufState& other) { + if (other.armed) { + memcpy(&buf, &other.buf, sizeof(buf)); + armed = true; + } + } + + JmpBufState& operator=(const JmpBufState& other) { + if (&other != this) { + if (other.armed) { + memcpy(&buf, &other.buf, sizeof(buf)); + armed = true; + } else { + armed = false; + } + } + return *this; + } +}; + +class ScopedJmpBuf final { + public: + ScopedJmpBuf(const JmpBufState& prev) : mPrev(prev) {} + ~ScopedJmpBuf(); + + ScopedJmpBuf(const ScopedJmpBuf&) = delete; + + private: + const JmpBufState& mPrev; +}; + +#define SCOPED_SIGBUS_HANDLER_CONDITIONAL(condition, code) \ + (void)incfs::SignalHandler::instance(); \ + auto& tlsBuf_macro = incfs::SignalHandler::mJmpBuf; \ + incfs::JmpBufState oldBuf_macro = tlsBuf_macro; \ + const bool condition_macro_val = (condition); \ + if (condition_macro_val && setjmp(tlsBuf_macro.buf) != 0) { \ + ALOGI("%s: handling SIGBUS at line %d", __func__, __LINE__); \ + tlsBuf_macro = oldBuf_macro; \ + do { \ + code; \ + } while (0); \ + LOG_ALWAYS_FATAL("%s(): signal handler was supposed to return", __func__); \ + } \ + tlsBuf_macro.armed |= (condition_macro_val); \ + incfs::ScopedJmpBuf oldBufRestore_macro(oldBuf_macro) + +#define SCOPED_SIGBUS_HANDLER(code) \ + SCOPED_SIGBUS_HANDLER_CONDITIONAL(true, code) + +class SignalHandler final { + public: + static SignalHandler& instance(); + + private: + SignalHandler(); + inline static struct sigaction mOldSigaction = {}; + + static void handler(int sig, siginfo_t* info, void* ucontext); + + public: + inline static thread_local JmpBufState mJmpBuf = {}; +}; + +} // namespace incfs + +#endif
diff --git a/incfs_support/include/incfs_support/util.h b/incfs_support/include/incfs_support/util.h new file mode 100644 index 0000000..aa4c950 --- /dev/null +++ b/incfs_support/include/incfs_support/util.h
@@ -0,0 +1,30 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +namespace incfs::util { + +// Some tools useful for writing hardened code + +// Clear the passed container and make sure it frees all allocated memory. +// Useful for signal handling code where any remaining memory would leak. +template <class Container> +void clearAndFree(Container& c) { + Container().swap(c); +} + +} // namespace incfs::util \ No newline at end of file
diff --git a/incfs_support/signal_handling.cpp b/incfs_support/signal_handling.cpp new file mode 100644 index 0000000..39cb0c6 --- /dev/null +++ b/incfs_support/signal_handling.cpp
@@ -0,0 +1,93 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "incfs_support/signal_handling.h" + +#if defined(__BIONIC__) && !INCFS_SUPPORT_DISABLED + +#include <errno.h> + +namespace incfs { + +static void enableSignal(int code) { + sigset_t allowed; + sigemptyset(&allowed); + sigaddset(&allowed, code); + pthread_sigmask(SIG_UNBLOCK, &allowed, nullptr); +} + +ScopedJmpBuf::~ScopedJmpBuf() { SignalHandler::mJmpBuf = mPrev; } + +SignalHandler& SignalHandler::instance() { + static SignalHandler self; + return self; +} + +SignalHandler::SignalHandler() { + const struct sigaction action = { + .sa_sigaction = &handler, + .sa_flags = SA_SIGINFO, + }; + if (sigaction(SIGBUS, &action, &mOldSigaction)) { + LOG_ALWAYS_FATAL("sigaction(SIGBUS) failed: %d", errno); + // not much can be done now + return; + } + + // ensure SIGBUS is unblocked, so the process won't get insta-killed + enableSignal(SIGBUS); +} + +void SignalHandler::handler(int sig, siginfo_t* info, void* ucontext) { + if (sig != SIGBUS) { + LOG_FATAL("SIGBUS handler called for unexpected signal %d", sig); + return; + } + + if (!mJmpBuf.armed) { + // No error handler installed - run the previous one + if (mOldSigaction.sa_handler == SIG_DFL) { + // reset the action to default and re-raise the signal. It will kill the + // process + signal(sig, SIG_DFL); + raise(sig); + return; + } + if (mOldSigaction.sa_handler == SIG_IGN) { + // ignoring SIGBUS won't help us much, as we'll get back right here after + // retrying. + return; + } + if (mOldSigaction.sa_flags & SA_SIGINFO) { + mOldSigaction.sa_sigaction(sig, info, ucontext); + } else { + mOldSigaction.sa_handler(sig); + } + // Returning from a signal handler for SIGBUS is undefined, but if it + // happens better to at least forward that undefined thing further. Who are + // we to argue with the user? + return; + } + + // restore SIGBUS as signal handling blocks it before running the callback + enableSignal(SIGBUS); + + longjmp(mJmpBuf.buf, 1); +} + +} // namespace incfs + +#endif
diff --git a/include/ziparchive/zip_archive.h b/include/ziparchive/zip_archive.h index 50d1951..3843026 100644 --- a/include/ziparchive/zip_archive.h +++ b/include/ziparchive/zip_archive.h
@@ -296,6 +296,12 @@ int32_t ExtractToMemory(ZipArchiveHandle archive, const ZipEntry* entry, uint8_t* begin, size_t size); +// +// This gets defined for the version of the library that need to control all +// code accessing the zip file. Details in incfs_support/signal_handling.h +// +#if !ZIPARCHIVE_DISABLE_CALLBACK_API + #if !defined(_WIN32) typedef bool (*ProcessZipEntryFunction)(const uint8_t* buf, size_t buf_size, void* cookie); @@ -307,7 +313,9 @@ ProcessZipEntryFunction func, void* cookie); int32_t ProcessZipEntryContents(ZipArchiveHandle archive, const ZipEntry64* entry, ProcessZipEntryFunction func, void* cookie); -#endif +#endif // !defined(_WIN32) + +#endif // !ZIPARCHIVE_DISABLE_CALLBACK_API namespace zip_archive { @@ -337,6 +345,12 @@ void operator=(const Reader&) = delete; }; +// +// This gets defined for the version of the library that need to control all +// code accessing the zip file. Details in incfs_support/signal_handling.h +// +#if !ZIPARCHIVE_DISABLE_CALLBACK_API + /** * Uncompress a given zip entry to given |writer|. * @@ -345,6 +359,8 @@ int32_t ExtractToWriter(ZipArchiveHandle handle, const ZipEntry64* entry, zip_archive::Writer* writer); +#endif // !ZIPARCHIVE_DISABLE_CALLBACK_API + /* * Inflates the first |compressed_length| bytes of |reader| to a given |writer|. * |crc_out| is set to the CRC32 checksum of the uncompressed data. @@ -355,7 +371,12 @@ * * If |crc_out| is not nullptr, it is set to the crc32 checksum of the * uncompressed data. + * + * NOTE: in the IncFS version of the library this function remains + * unprotected, because the data |reader| is supplying is under the full reader's + * control; it's the reader's duty to ensure it is available and OK to access. */ int32_t Inflate(const Reader& reader, const uint64_t compressed_length, const uint64_t uncompressed_length, Writer* writer, uint64_t* crc_out); + } // namespace zip_archive
diff --git a/zip_archive.cc b/zip_archive.cc index fe1baa1..8d87f6f 100644 --- a/zip_archive.cc +++ b/zip_archive.cc
@@ -33,6 +33,7 @@ #include <memory> #include <optional> +#include <span> #include <vector> #if defined(__APPLE__) @@ -51,11 +52,13 @@ #include <android-base/strings.h> #include <android-base/utf8.h> #include <log/log.h> -#include "zlib.h" #include "entry_name_utils-inl.h" +#include "incfs_support/signal_handling.h" +#include "incfs_support/util.h" #include "zip_archive_common.h" #include "zip_archive_private.h" +#include "zlib.h" // Used to turn on crc checks - verify that the content CRC matches the values // specified in the local file header and the central directory. @@ -209,10 +212,12 @@ return kSuccess; } -static ZipError FindCentralDirectoryInfo(const char* debug_file_name, ZipArchive* archive, - off64_t file_length, uint32_t read_amount, +static ZipError FindCentralDirectoryInfo(const char* debug_file_name, + ZipArchive* archive, + off64_t file_length, + std::span<uint8_t> scan_buffer, CentralDirectoryInfo* cdInfo) { - std::vector<uint8_t> scan_buffer(read_amount); + const auto read_amount = static_cast<uint32_t>(scan_buffer.size()); const off64_t search_start = file_length - read_amount; if (!archive->mapped_zip.ReadAtOffset(scan_buffer.data(), read_amount, search_start)) { @@ -322,12 +327,21 @@ } CentralDirectoryInfo cdInfo = {}; - if (auto result = - FindCentralDirectoryInfo(debug_file_name, archive, file_length, read_amount, &cdInfo); + std::vector<uint8_t> scan_buffer(read_amount); + + SCOPED_SIGBUS_HANDLER({ + incfs::util::clearAndFree(scan_buffer); + return kIoError; + }); + + if (auto result = FindCentralDirectoryInfo(debug_file_name, archive, + file_length, scan_buffer, &cdInfo); result != kSuccess) { return result; } + scan_buffer.clear(); + if (cdInfo.num_records == 0) { #if defined(__ANDROID__) ALOGW("Zip: empty archive?"); @@ -465,6 +479,8 @@ * Returns 0 on success. */ static ZipError ParseZipArchive(ZipArchive* archive) { + SCOPED_SIGBUS_HANDLER(return kIoError); + const uint8_t* const cd_ptr = archive->central_directory.GetBasePtr(); const size_t cd_length = archive->central_directory.GetMapLength(); const uint64_t num_entries = archive->num_entries; @@ -647,6 +663,8 @@ } static int32_t ValidateDataDescriptor(MappedZipFile& mapped_zip, const ZipEntry64* entry) { + SCOPED_SIGBUS_HANDLER(return kIoError); + // Maximum possible size for data descriptor: 2 * 4 + 2 * 8 = 24 bytes // The zip format doesn't specify the size of data descriptor. But we won't read OOB here even // if the descriptor isn't present. Because the size cd + eocd in the end of the zipfile is @@ -697,6 +715,14 @@ static int32_t FindEntry(const ZipArchive* archive, std::string_view entryName, const uint64_t nameOffset, ZipEntry64* data) { + std::vector<uint8_t> name_buf; + std::vector<uint8_t> local_extra_field; + SCOPED_SIGBUS_HANDLER({ + incfs::util::clearAndFree(name_buf); + incfs::util::clearAndFree(local_extra_field); + return kIoError; + }); + // Recover the start of the central directory entry from the filename // pointer. The filename is the first entry past the fixed-size data, // so we can just subtract back from that. @@ -794,7 +820,7 @@ return kInvalidOffset; } - std::vector<uint8_t> name_buf(name_length); + name_buf.resize(name_length); if (!archive->mapped_zip.ReadAtOffset(name_buf.data(), name_buf.size(), name_offset)) { ALOGW("Zip: failed reading lfh name from offset %" PRId64, static_cast<int64_t>(name_offset)); return kIoError; @@ -821,7 +847,7 @@ return kInvalidOffset; } - std::vector<uint8_t> local_extra_field(lfh_extra_field_size); + local_extra_field.resize(lfh_extra_field_size); if (!archive->mapped_zip.ReadAtOffset(local_extra_field.data(), lfh_extra_field_size, lfh_extra_field_offset)) { ALOGW("Zip: failed reading lfh extra field from offset %" PRId64, lfh_extra_field_offset); @@ -956,7 +982,7 @@ } archive->cd_entry_map->ResetIteration(); - *cookie_ptr = new IterationHandle(archive, matcher); + *cookie_ptr = new IterationHandle(archive, std::move(matcher)); return 0; } @@ -1046,6 +1072,8 @@ return kInvalidHandle; } + SCOPED_SIGBUS_HANDLER(return kIoError); + auto entry = archive->cd_entry_map->Next(archive->central_directory.GetBasePtr()); while (entry != std::pair<std::string_view, uint64_t>()) { const auto [entry_name, offset] = entry; @@ -1068,15 +1096,16 @@ // the data appended to it. class MemoryWriter : public zip_archive::Writer { public: - static std::unique_ptr<MemoryWriter> Create(uint8_t* buf, size_t size, const ZipEntry64* entry) { + static std::optional<MemoryWriter> Create(uint8_t* buf, size_t size, + const ZipEntry64* entry) { const uint64_t declared_length = entry->uncompressed_length; if (declared_length > size) { ALOGW("Zip: file size %" PRIu64 " is larger than the buffer size %zu.", declared_length, size); - return nullptr; + return {}; } - return std::unique_ptr<MemoryWriter>(new MemoryWriter(buf, size)); + return std::make_optional<MemoryWriter>(buf, size); } virtual bool Append(uint8_t* buf, size_t buf_size) override { @@ -1091,9 +1120,9 @@ return true; } - private: MemoryWriter(uint8_t* buf, size_t size) : Writer(), buf_(buf), size_(size), bytes_written_(0) {} + private: uint8_t* const buf_{nullptr}; const size_t size_; size_t bytes_written_; @@ -1109,18 +1138,18 @@ // is truncated to the correct length (no truncation if |fd| references a // block device). // - // Returns a valid FileWriter on success, |nullptr| if an error occurred. - static std::unique_ptr<FileWriter> Create(int fd, const ZipEntry64* entry) { + // Returns a valid FileWriter on success, |nullopt| if an error occurred. + static std::optional<FileWriter> Create(int fd, const ZipEntry64* entry) { const uint64_t declared_length = entry->uncompressed_length; const off64_t current_offset = lseek64(fd, 0, SEEK_CUR); if (current_offset == -1) { ALOGW("Zip: unable to seek to current location on fd %d: %s", fd, strerror(errno)); - return nullptr; + return {}; } if (declared_length > SIZE_MAX || declared_length > INT64_MAX) { ALOGW("Zip: file size %" PRIu64 " is too large to extract.", declared_length); - return nullptr; + return {}; } #if defined(__linux__) @@ -1138,7 +1167,7 @@ if (result == -1 && errno == ENOSPC) { ALOGW("Zip: unable to allocate %" PRIu64 " bytes at offset %" PRId64 ": %s", declared_length, static_cast<int64_t>(current_offset), strerror(errno)); - return nullptr; + return {}; } } #endif // __linux__ @@ -1146,7 +1175,7 @@ struct stat sb; if (fstat(fd, &sb) == -1) { ALOGW("Zip: unable to fstat file: %s", strerror(errno)); - return nullptr; + return {}; } // Block device doesn't support ftruncate(2). @@ -1155,18 +1184,11 @@ if (result == -1) { ALOGW("Zip: unable to truncate file to %" PRId64 ": %s", static_cast<int64_t>(declared_length + current_offset), strerror(errno)); - return nullptr; + return {}; } } - return std::unique_ptr<FileWriter>(new FileWriter(fd, declared_length)); - } - - FileWriter(FileWriter&& other) noexcept - : fd_(other.fd_), - declared_length_(other.declared_length_), - total_bytes_written_(other.total_bytes_written_) { - other.fd_ = -1; + return std::make_optional<FileWriter>(fd, declared_length); } virtual bool Append(uint8_t* buf, size_t buf_size) override { @@ -1186,7 +1208,6 @@ return result; } - private: explicit FileWriter(const int fd = -1, const uint64_t declared_length = 0) : Writer(), fd_(fd), @@ -1195,6 +1216,7 @@ CHECK_LE(declared_length, SIZE_MAX); } + private: int fd_; const size_t declared_length_; size_t total_bytes_written_; @@ -1230,8 +1252,13 @@ Reader::~Reader() {} Writer::~Writer() {} -int32_t Inflate(const Reader& reader, const uint64_t compressed_length, - const uint64_t uncompressed_length, Writer* writer, uint64_t* crc_out) { +} // namespace zip_archive + +template <bool OnIncfs> +static int32_t inflateImpl(const zip_archive::Reader& reader, + const uint64_t compressed_length, + const uint64_t uncompressed_length, + zip_archive::Writer* writer, uint64_t* crc_out) { const size_t kBufSize = 32768; std::vector<uint8_t> read_buf(kBufSize); std::vector<uint8_t> write_buf(kBufSize); @@ -1272,6 +1299,13 @@ std::unique_ptr<z_stream, decltype(zstream_deleter)> zstream_guard(&zstream, zstream_deleter); + SCOPED_SIGBUS_HANDLER_CONDITIONAL(OnIncfs, { + zstream_guard.reset(); + incfs::util::clearAndFree(read_buf); + incfs::util::clearAndFree(write_buf); + return kIoError; + }); + const bool compute_crc = (crc_out != nullptr); uLong crc = 0; uint64_t remaining_bytes = compressed_length; @@ -1337,14 +1371,13 @@ return 0; } -} // namespace zip_archive static int32_t InflateEntryToWriter(MappedZipFile& mapped_zip, const ZipEntry64* entry, zip_archive::Writer* writer, uint64_t* crc_out) { const EntryReader reader(mapped_zip, entry); - return zip_archive::Inflate(reader, entry->compressed_length, entry->uncompressed_length, writer, - crc_out); + return inflateImpl<true>(reader, entry->compressed_length, + entry->uncompressed_length, writer, crc_out); } static int32_t CopyEntryToWriter(MappedZipFile& mapped_zip, const ZipEntry64* entry, @@ -1352,6 +1385,11 @@ static const uint32_t kBufSize = 32768; std::vector<uint8_t> buf(kBufSize); + SCOPED_SIGBUS_HANDLER({ + incfs::util::clearAndFree(buf); + return kIoError; + }); + const uint64_t length = entry->uncompressed_length; uint64_t count = 0; uLong crc = 0; @@ -1386,10 +1424,8 @@ return 0; } -namespace zip_archive { - -int32_t ExtractToWriter(ZipArchiveHandle handle, const ZipEntry64* entry, - zip_archive::Writer* writer) { +static int32_t extractToWriter(ZipArchiveHandle handle, const ZipEntry64* entry, + zip_archive::Writer* writer) { const uint16_t method = entry->method; // this should default to kUnknownCompressionMethod. @@ -1419,8 +1455,6 @@ return return_value; } -} // namespace zip_archive - int32_t ExtractToMemory(ZipArchiveHandle archive, const ZipEntry* entry, uint8_t* begin, size_t size) { ZipEntry64 entry64(*entry); @@ -1434,7 +1468,7 @@ return kIoError; } - return ExtractToWriter(archive, entry, writer.get()); + return extractToWriter(archive, entry, &writer.value()); } int32_t ExtractEntryToFile(ZipArchiveHandle archive, const ZipEntry* entry, int fd) { @@ -1448,7 +1482,7 @@ return kIoError; } - return ExtractToWriter(archive, entry, writer.get()); + return extractToWriter(archive, entry, &writer.value()); } int GetFileDescriptor(const ZipArchiveHandle archive) { @@ -1459,7 +1493,13 @@ return archive->mapped_zip.GetFileOffset(); } -#if !defined(_WIN32) +// +// ZIPARCHIVE_DISABLE_CALLBACK_API disables all APIs that accept user callbacks. +// It gets defined for the incfs-supporting version of libziparchive, where one +// has to control all the code accessing the archive. See more at +// incfs_support/signal_handling.h +// +#if !ZIPARCHIVE_DISABLE_CALLBACK_API && !defined(_WIN32) class ProcessWriter : public zip_archive::Writer { public: ProcessWriter(ProcessZipEntryFunction func, void* cookie) @@ -1483,10 +1523,10 @@ int32_t ProcessZipEntryContents(ZipArchiveHandle archive, const ZipEntry64* entry, ProcessZipEntryFunction func, void* cookie) { ProcessWriter writer(func, cookie); - return ExtractToWriter(archive, entry, &writer); + return extractToWriter(archive, entry, &writer); } -#endif //! defined(_WIN32) +#endif // !ZIPARCHIVE_DISABLE_CALLBACK_API && !defined(_WIN32) int MappedZipFile::GetFileDescriptor() const { if (!has_fd_) { @@ -1624,3 +1664,29 @@ return t; } + +namespace zip_archive { + +int32_t Inflate(const Reader& reader, const uint64_t compressed_length, + const uint64_t uncompressed_length, Writer* writer, + uint64_t* crc_out) { + return inflateImpl<false>(reader, compressed_length, uncompressed_length, + writer, crc_out); +} + +// +// ZIPARCHIVE_DISABLE_CALLBACK_API disables all APIs that accept user callbacks. +// It gets defined for the incfs-supporting version of libziparchive, where one +// has to control all the code accessing the archive. See more at +// incfs_support/signal_handling.h +// +#if !ZIPARCHIVE_DISABLE_CALLBACK_API + +int32_t ExtractToWriter(ZipArchiveHandle handle, const ZipEntry64* entry, + zip_archive::Writer* writer) { + return extractToWriter(handle, entry, writer); +} + +#endif // !ZIPARCHIVE_DISABLE_CALLBACK_API + +} // namespace zip_archive
diff --git a/zip_archive_common.h b/zip_archive_common.h index d461856..7605953 100644 --- a/zip_archive_common.h +++ b/zip_archive_common.h
@@ -60,8 +60,9 @@ // Length of the central directory comment. uint16_t comment_length; - private: EocdRecord() = default; + + private: DISALLOW_COPY_AND_ASSIGN(EocdRecord); } __attribute__((packed)); @@ -115,8 +116,9 @@ // beginning of this archive. uint32_t local_file_header_offset; - private: CentralDirectoryRecord() = default; + + private: DISALLOW_COPY_AND_ASSIGN(CentralDirectoryRecord); } __attribute__((packed)); @@ -154,8 +156,9 @@ // will appear immediately after the entry file name. uint16_t extra_field_length; - private: LocalFileHeader() = default; + + private: DISALLOW_COPY_AND_ASSIGN(LocalFileHeader); } __attribute__((packed)); @@ -179,8 +182,9 @@ // in the zip file. uint64_t uncompressed_size; - private: DataDescriptor() = default; + + private: DISALLOW_COPY_AND_ASSIGN(DataDescriptor); }; @@ -199,8 +203,9 @@ // spans a single disk only. uint32_t num_of_disks; - private: Zip64EocdLocator() = default; + + private: DISALLOW_COPY_AND_ASSIGN(Zip64EocdLocator); } __attribute__((packed)); @@ -235,8 +240,9 @@ // the file. uint64_t cd_start_offset; - private: Zip64EocdRecord() = default; + + private: DISALLOW_COPY_AND_ASSIGN(Zip64EocdRecord); } __attribute__((packed)); @@ -262,8 +268,9 @@ // This implementation assumes that each archive spans a single disk only. So // the disk_number is not used. // uint32_t disk_num; - private: Zip64ExtendedInfo() = default; + + private: DISALLOW_COPY_AND_ASSIGN(Zip64ExtendedInfo); };