From: Leslie P. Polzer Date: Mon, 22 Dec 2025 04:26:57 +0000 (+0000) Subject: Refactor fuzzers: add shared header, remove system() call X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=544b07541bf398ed490ed6eebdece8fd5ff7422f;p=thirdparty%2Flibarchive.git Refactor fuzzers: add shared header, remove system() call - Add fuzz_helpers.h with shared Buffer, reader_callback, DataConsumer - Replace system("rm -rf") with nftw-based remove_directory_tree() - Refactor entry, tar, write_disk fuzzers to use shared helpers - Reduces code duplication and improves maintainability --- diff --git a/contrib/oss-fuzz/fuzz_helpers.h b/contrib/oss-fuzz/fuzz_helpers.h new file mode 100644 index 000000000..bbdfdaf3a --- /dev/null +++ b/contrib/oss-fuzz/fuzz_helpers.h @@ -0,0 +1,156 @@ +// Copyright 2024 Google LLC +// +// 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. + +#ifndef LIBARCHIVE_FUZZ_HELPERS_H_ +#define LIBARCHIVE_FUZZ_HELPERS_H_ + +#include +#include +#include +#include +#include + +#include "archive.h" + +// Default maximum input size for fuzzers +static constexpr size_t kDefaultMaxInputSize = 256 * 1024; // 256KB + +// Buffer structure for archive reading callbacks +struct Buffer { + const uint8_t* data; + size_t size; + size_t pos; +}; + +// Archive read callback function +static la_ssize_t reader_callback(struct archive* a, void* client_data, + const void** buffer) { + (void)a; + Buffer* buf = static_cast(client_data); + + if (buf->pos >= buf->size) { + return 0; // EOF + } + + *buffer = buf->data + buf->pos; + size_t remaining = buf->size - buf->pos; + buf->pos = buf->size; // Consume all remaining data + return static_cast(remaining); +} + +// Helper class for consuming fuzz data in structured ways +class DataConsumer { + public: + DataConsumer(const uint8_t* data, size_t size) + : data_(data), size_(size), pos_(0) {} + + bool empty() const { return pos_ >= size_; } + size_t remaining() const { return size_ - pos_; } + + uint8_t consume_byte() { + if (pos_ >= size_) return 0; + return data_[pos_++]; + } + + uint16_t consume_u16() { + uint16_t val = 0; + if (pos_ + 2 <= size_) { + val = static_cast(data_[pos_]) | + (static_cast(data_[pos_ + 1]) << 8); + pos_ += 2; + } + return val; + } + + uint32_t consume_u32() { + uint32_t val = 0; + if (pos_ + 4 <= size_) { + val = static_cast(data_[pos_]) | + (static_cast(data_[pos_ + 1]) << 8) | + (static_cast(data_[pos_ + 2]) << 16) | + (static_cast(data_[pos_ + 3]) << 24); + pos_ += 4; + } + return val; + } + + int64_t consume_i64() { + int64_t val = 0; + if (pos_ + 8 <= size_) { + for (int i = 0; i < 8; i++) { + val |= static_cast(data_[pos_ + i]) << (8 * i); + } + pos_ += 8; + } + return val; + } + + // Consume a null-terminated string up to max_len characters + // Returns pointer to internal buffer (valid until next consume_string call) + const char* consume_string(size_t max_len) { + if (max_len > sizeof(string_buf_) - 1) { + max_len = sizeof(string_buf_) - 1; + } + size_t avail = size_ - pos_; + size_t len = (avail < max_len) ? avail : max_len; + size_t actual_len = 0; + + while (actual_len < len && pos_ < size_) { + char c = static_cast(data_[pos_++]); + if (c == '\0') break; + string_buf_[actual_len++] = c; + } + string_buf_[actual_len] = '\0'; + return string_buf_; + } + + // Consume raw bytes into a buffer + size_t consume_bytes(void* out, size_t len) { + size_t avail = size_ - pos_; + size_t to_copy = (avail < len) ? avail : len; + if (to_copy > 0) { + memcpy(out, data_ + pos_, to_copy); + pos_ += to_copy; + } + return to_copy; + } + + // Get remaining data as a buffer + const uint8_t* remaining_data() const { + return data_ + pos_; + } + + private: + const uint8_t* data_; + size_t size_; + size_t pos_; + char string_buf_[512]; +}; + +// Callback for nftw to remove files/directories +static int remove_callback(const char* fpath, const struct stat* sb, + int typeflag, struct FTW* ftwbuf) { + (void)sb; + (void)typeflag; + (void)ftwbuf; + return remove(fpath); +} + +// Recursively remove a directory tree (safer than system("rm -rf ...")) +static int remove_directory_tree(const char* path) { + // nftw with FTW_DEPTH processes directory contents before the directory itself + return nftw(path, remove_callback, 64, FTW_DEPTH | FTW_PHYS); +} + +#endif // LIBARCHIVE_FUZZ_HELPERS_H_ diff --git a/contrib/oss-fuzz/libarchive_entry_fuzzer.cc b/contrib/oss-fuzz/libarchive_entry_fuzzer.cc index 7a6c1860e..d8aa8b51e 100644 --- a/contrib/oss-fuzz/libarchive_entry_fuzzer.cc +++ b/contrib/oss-fuzz/libarchive_entry_fuzzer.cc @@ -9,64 +9,10 @@ #include "archive.h" #include "archive_entry.h" +#include "fuzz_helpers.h" static constexpr size_t kMaxInputSize = 64 * 1024; // 64KB -// FuzzedDataProvider-like helper for consuming bytes -class DataConsumer { -public: - DataConsumer(const uint8_t *data, size_t size) : data_(data), size_(size), pos_(0) { - memset(string_buf_, 0, sizeof(string_buf_)); - } - - bool empty() const { return pos_ >= size_; } - - uint8_t consume_byte() { - if (pos_ >= size_) return 0; - return data_[pos_++]; - } - - uint32_t consume_uint32() { - uint32_t val = 0; - for (int i = 0; i < 4 && pos_ < size_; i++) { - val |= static_cast(data_[pos_++]) << (i * 8); - } - return val; - } - - int64_t consume_int64() { - int64_t val = 0; - for (int i = 0; i < 8 && pos_ < size_; i++) { - val |= static_cast(data_[pos_++]) << (i * 8); - } - return val; - } - - const char* consume_string(size_t max_len) { - if (max_len > sizeof(string_buf_) - 1) max_len = sizeof(string_buf_) - 1; - size_t avail = size_ - pos_; - size_t len = (avail < max_len) ? avail : max_len; - - // Copy to internal buffer and null-terminate - size_t actual_len = 0; - while (actual_len < len && pos_ < size_) { - char c = static_cast(data_[pos_++]); - if (c == '\0') break; - string_buf_[actual_len++] = c; - } - string_buf_[actual_len] = '\0'; - return string_buf_; - } - - size_t remaining() const { return size_ - pos_; } - -private: - const uint8_t *data_; - size_t size_; - size_t pos_; - char string_buf_[512]; -}; - extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { if (len == 0 || len > kMaxInputSize) { return 0; @@ -81,14 +27,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { // Set basic entry properties archive_entry_set_pathname(entry, consumer.consume_string(256)); - archive_entry_set_size(entry, consumer.consume_int64()); - archive_entry_set_mode(entry, consumer.consume_uint32()); - archive_entry_set_uid(entry, consumer.consume_uint32()); - archive_entry_set_gid(entry, consumer.consume_uint32()); - archive_entry_set_mtime(entry, consumer.consume_int64(), 0); - archive_entry_set_atime(entry, consumer.consume_int64(), 0); - archive_entry_set_ctime(entry, consumer.consume_int64(), 0); - archive_entry_set_birthtime(entry, consumer.consume_int64(), 0); + archive_entry_set_size(entry, consumer.consume_i64()); + archive_entry_set_mode(entry, consumer.consume_u32()); + archive_entry_set_uid(entry, consumer.consume_u32()); + archive_entry_set_gid(entry, consumer.consume_u32()); + archive_entry_set_mtime(entry, consumer.consume_i64(), 0); + archive_entry_set_atime(entry, consumer.consume_i64(), 0); + archive_entry_set_ctime(entry, consumer.consume_i64(), 0); + archive_entry_set_birthtime(entry, consumer.consume_i64(), 0); // Set various string fields archive_entry_set_uname(entry, consumer.consume_string(64)); @@ -98,9 +44,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { // Exercise ACL functions (low coverage targets) int acl_type = consumer.consume_byte() & 0x0F; - int acl_permset = consumer.consume_uint32(); + int acl_permset = consumer.consume_u32(); int acl_tag = consumer.consume_byte() & 0x0F; - int acl_qual = consumer.consume_uint32(); + int acl_qual = consumer.consume_u32(); const char *acl_name = consumer.consume_string(64); archive_entry_acl_add_entry(entry, acl_type, acl_permset, acl_tag, acl_qual, acl_name); @@ -108,9 +54,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { // Add more ACL entries based on remaining data while (!consumer.empty() && consumer.remaining() > 10) { acl_type = consumer.consume_byte() & 0x0F; - acl_permset = consumer.consume_uint32(); + acl_permset = consumer.consume_u32(); acl_tag = consumer.consume_byte() & 0x0F; - acl_qual = consumer.consume_uint32(); + acl_qual = consumer.consume_u32(); acl_name = consumer.consume_string(32); archive_entry_acl_add_entry(entry, acl_type, acl_permset, acl_tag, acl_qual, acl_name); } diff --git a/contrib/oss-fuzz/libarchive_tar_fuzzer.cc b/contrib/oss-fuzz/libarchive_tar_fuzzer.cc index 48e9d700b..ef4ce5fe3 100644 --- a/contrib/oss-fuzz/libarchive_tar_fuzzer.cc +++ b/contrib/oss-fuzz/libarchive_tar_fuzzer.cc @@ -8,24 +8,10 @@ #include "archive.h" #include "archive_entry.h" +#include "fuzz_helpers.h" static constexpr size_t kMaxInputSize = 512 * 1024; -struct Buffer { - const uint8_t *buf; - size_t len; -}; - -static ssize_t reader_callback(struct archive *a, void *client_data, - const void **block) { - (void)a; - Buffer *buffer = reinterpret_cast(client_data); - *block = buffer->buf; - ssize_t len = buffer->len; - buffer->len = 0; - return len; -} - extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { if (len == 0 || len > kMaxInputSize) { return 0; @@ -43,7 +29,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { // Enable various TAR options archive_read_set_options(a, "tar:read_concatenated_archives,tar:mac-ext"); - Buffer buffer = {buf, len}; + Buffer buffer = {buf, len, 0}; if (archive_read_open(a, &buffer, NULL, reader_callback, NULL) != ARCHIVE_OK) { archive_read_free(a); return 0; diff --git a/contrib/oss-fuzz/libarchive_write_disk_fuzzer.cc b/contrib/oss-fuzz/libarchive_write_disk_fuzzer.cc index 1d54c7510..056bd1639 100644 --- a/contrib/oss-fuzz/libarchive_write_disk_fuzzer.cc +++ b/contrib/oss-fuzz/libarchive_write_disk_fuzzer.cc @@ -9,77 +9,15 @@ #include #include #include -#include #include "archive.h" #include "archive_entry.h" +#include "fuzz_helpers.h" static constexpr size_t kMaxInputSize = 64 * 1024; static char g_temp_dir[256] = {0}; -class DataConsumer { -public: - DataConsumer(const uint8_t *data, size_t size) : data_(data), size_(size), pos_(0) { - memset(string_buf_, 0, sizeof(string_buf_)); - } - - bool empty() const { return pos_ >= size_; } - - uint8_t consume_byte() { - if (pos_ >= size_) return 0; - return data_[pos_++]; - } - - uint32_t consume_uint32() { - uint32_t val = 0; - for (int i = 0; i < 4 && pos_ < size_; i++) { - val |= static_cast(data_[pos_++]) << (i * 8); - } - return val; - } - - int64_t consume_int64() { - int64_t val = 0; - for (int i = 0; i < 8 && pos_ < size_; i++) { - val |= static_cast(data_[pos_++]) << (i * 8); - } - return val; - } - - const char* consume_path(size_t max_len) { - if (max_len > sizeof(string_buf_) - 1) max_len = sizeof(string_buf_) - 1; - size_t avail = size_ - pos_; - size_t len = (avail < max_len) ? avail : max_len; - - size_t actual_len = 0; - while (actual_len < len && pos_ < size_) { - char c = static_cast(data_[pos_++]); - if (c == '\0') break; - string_buf_[actual_len++] = c; - } - string_buf_[actual_len] = '\0'; - return string_buf_; - } - - const uint8_t* consume_bytes(size_t *out_len, size_t max_len) { - size_t avail = size_ - pos_; - size_t len = (avail < max_len) ? avail : max_len; - const uint8_t *ptr = data_ + pos_; - pos_ += len; - *out_len = len; - return ptr; - } - - size_t remaining() const { return size_ - pos_; } - -private: - const uint8_t *data_; - size_t size_; - size_t pos_; - char string_buf_[256]; -}; - extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { (void)argc; (void)argv; @@ -130,7 +68,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { // Build a safe path within our temp directory char safe_path[512]; - const char *name = consumer.consume_path(32); + const char *name = consumer.consume_string(32); snprintf(safe_path, sizeof(safe_path), "%s/%s", g_temp_dir, name); // Sanitize path to prevent traversal @@ -156,16 +94,16 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { archive_entry_set_uid(entry, 1000); archive_entry_set_gid(entry, 1000); - archive_entry_set_mtime(entry, consumer.consume_int64(), 0); + archive_entry_set_mtime(entry, consumer.consume_i64(), 0); // Write the entry header if (archive_write_header(disk, entry) == ARCHIVE_OK) { if (S_ISREG(mode)) { - size_t data_len; - const uint8_t *data = consumer.consume_bytes(&data_len, 256); + uint8_t data_buf[256]; + size_t data_len = consumer.consume_bytes(data_buf, 256); archive_entry_set_size(entry, data_len); if (data_len > 0) { - archive_write_data(disk, data, data_len); + archive_write_data(disk, data_buf, data_len); } } archive_write_finish_entry(disk); @@ -178,10 +116,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { archive_write_close(disk); archive_write_free(disk); - // Clean up extracted files - char cmd[600]; - snprintf(cmd, sizeof(cmd), "rm -rf %s/* 2>/dev/null", g_temp_dir); - (void)system(cmd); + // Clean up extracted files using nftw (safer than system()) + remove_directory_tree(g_temp_dir); + // Recreate the temp directory for next iteration + mkdir(g_temp_dir, 0700); return 0; }