From 079de65a4a6e8259cfbd26bc15e2246089c7bafa Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Tue, 20 Dec 2022 17:45:26 +0100 Subject: [PATCH] Factor out realloc handling to a separate helper function --- lib/CMakeLists.txt | 1 + lib/zip_add_entry.c | 21 ++++--------- lib/zip_dirent.c | 19 ++++-------- lib/zip_realloc.c | 66 +++++++++++++++++++++++++++++++++++++++++ lib/zip_source_buffer.c | 32 ++++++++++---------- lib/zip_source_window.c | 12 ++++---- lib/zipint.h | 3 ++ 7 files changed, 103 insertions(+), 51 deletions(-) create mode 100644 lib/zip_realloc.c diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index b4df8b06..f4e594d3 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -60,6 +60,7 @@ add_library(zip zip_open.c zip_pkware.c zip_progress.c + zip_realloc.c zip_rename.c zip_replace.c zip_set_archive_comment.c diff --git a/lib/zip_add_entry.c b/lib/zip_add_entry.c index bf12dd54..035f1bc9 100644 --- a/lib/zip_add_entry.c +++ b/lib/zip_add_entry.c @@ -44,10 +44,8 @@ _zip_add_entry(zip_t *za) { zip_uint64_t idx; if (za->nentry + 1 >= za->nentry_alloc) { - zip_entry_t *rentries; - zip_uint64_t nalloc = za->nentry_alloc; - zip_uint64_t additional_entries = 2 * nalloc; - zip_uint64_t realloc_size; + zip_uint64_t additional_entries = 2 * za->nentry_alloc; + int result; if (additional_entries < 16) { additional_entries = 16; @@ -55,21 +53,12 @@ _zip_add_entry(zip_t *za) { else if (additional_entries > 1024) { additional_entries = 1024; } - /* neither + nor * overflows can happen: nentry_alloc * sizeof(struct zip_entry) < UINT64_MAX */ - nalloc += additional_entries; - realloc_size = sizeof(struct zip_entry) * (size_t)nalloc; - if (sizeof(struct zip_entry) * (size_t)za->nentry_alloc > realloc_size) { - zip_error_set(&za->error, ZIP_ER_MEMORY, 0); + result = _zip_realloc(&za->entry, &za->nentry_alloc, sizeof(struct zip_entry), additional_entries); + if (result != ZIP_ER_OK) { + zip_error_set(&za->error, result, 0); return -1; } - rentries = (zip_entry_t *)realloc(za->entry, sizeof(struct zip_entry) * (size_t)nalloc); - if (!rentries) { - zip_error_set(&za->error, ZIP_ER_MEMORY, 0); - return -1; - } - za->entry = rentries; - za->nentry_alloc = nalloc; } idx = za->nentry++; diff --git a/lib/zip_dirent.c b/lib/zip_dirent.c index 76495530..36b22f51 100644 --- a/lib/zip_dirent.c +++ b/lib/zip_dirent.c @@ -88,30 +88,23 @@ bool _zip_cdir_grow(zip_cdir_t *cd, zip_uint64_t additional_entries, zip_error_t *error) { zip_uint64_t i, new_alloc; zip_entry_t *new_entry; + int result; if (additional_entries == 0) { return true; } - new_alloc = cd->nentry_alloc + additional_entries; - - if (new_alloc < additional_entries || new_alloc > SIZE_MAX / sizeof(*(cd->entry))) { - zip_error_set(error, ZIP_ER_MEMORY, 0); + result = _zip_realloc(&cd->entry, &cd->nentry_alloc, sizeof(*(cd->entry)), additional_entries); + if (result != ZIP_ER_OK) { + zip_error_set(error, result, 0); return false; } - if ((new_entry = (zip_entry_t *)realloc(cd->entry, sizeof(*(cd->entry)) * (size_t)new_alloc)) == NULL) { - zip_error_set(error, ZIP_ER_MEMORY, 0); - return false; - } - - cd->entry = new_entry; - - for (i = cd->nentry; i < new_alloc; i++) { + for (i = cd->nentry; i < cd->nentry_alloc; i++) { _zip_entry_init(cd->entry + i); } - cd->nentry = cd->nentry_alloc = new_alloc; + cd->nentry = cd->nentry_alloc; return true; } diff --git a/lib/zip_realloc.c b/lib/zip_realloc.c new file mode 100644 index 00000000..a79c4a7c --- /dev/null +++ b/lib/zip_realloc.c @@ -0,0 +1,66 @@ +/* + zip_realloc.c -- reallocate with additional elements + Copyright (C) 2009-2022 Dieter Baron and Thomas Klausner + + This file is part of libzip, a library to manipulate ZIP archives. + The authors can be contacted at + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions + are met: + 1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + 3. The names of the authors may not be used to endorse or promote + products derived from this software without specific prior + written permission. + + THIS SOFTWARE IS PROVIDED BY THE AUTHORS ``AS IS'' AND ANY EXPRESS + OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY + DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE + GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER + IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN + IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + + +#include + +#include "zipint.h" + +int +(_zip_realloc)(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements) { + void *old_memory = *inout_memory; + zip_uint64_t old_alloced = *inout_alloced; + zip_uint64_t new_alloced; + size_t new_size; + void *new_memory; + + if (additional_elements == 0) + additional_elements = 1; + + if (old_alloced > ZIP_UINT64_MAX - additional_elements) + return ZIP_ER_MEMORY; + + new_alloced = old_alloced + additional_elements; + + if (new_alloced > ((zip_uint64_t)SIZE_MAX) / element_size) + return ZIP_ER_MEMORY; + + new_size = new_alloced * element_size; + + if ((new_memory = realloc(old_memory, new_size)) == NULL) + return ZIP_ER_MEMORY; + + *inout_memory = new_memory; + *inout_alloced = new_alloced; + return ZIP_ER_OK; +} diff --git a/lib/zip_source_buffer.c b/lib/zip_source_buffer.c index 7bea464f..a4fc1489 100644 --- a/lib/zip_source_buffer.c +++ b/lib/zip_source_buffer.c @@ -429,30 +429,32 @@ static bool buffer_grow_fragments(buffer_t *buffer, zip_uint64_t capacity, zip_error_t *error) { zip_buffer_fragment_t *fragments; zip_uint64_t *offsets; + int result; + zip_uint64_t tmp_fragments_capacity, tmp_offsets_capacity, additional_elements; - if (capacity < buffer->fragments_capacity) { + if (capacity <= buffer->fragments_capacity) { return true; } - zip_uint64_t fragments_size = sizeof(buffer->fragments[0]) * capacity; - zip_uint64_t offsets_size = sizeof(buffer->fragment_offsets[0]) * (capacity + 1); - - if (capacity == ZIP_UINT64_MAX || fragments_size < capacity || fragments_size > SIZE_MAX|| offsets_size < capacity || offsets_size > SIZE_MAX) { - zip_error_set(error, ZIP_ER_MEMORY, 0); + tmp_fragments_capacity = buffer->fragments_capacity; + additional_elements = capacity - buffer->fragments_capacity; + result = _zip_realloc(&buffer->fragments, &tmp_fragments_capacity, sizeof(buffer->fragments[0]), additional_elements); + if (result != ZIP_ER_OK) { + zip_error_set(error, result, 0); return false; } - if ((fragments = realloc(buffer->fragments, (size_t)fragments_size)) == NULL) { - zip_error_set(error, ZIP_ER_MEMORY, 0); + /* technically it's a lie when we have no offsets at all in the + beginning, but it is of no consequence - the old capacity is + used only for computing the new capacity. */ + tmp_offsets_capacity = buffer->fragments_capacity + 1; + result = _zip_realloc(&buffer->fragment_offsets, &tmp_offsets_capacity, sizeof(buffer->fragment_offsets[0]), additional_elements); + if (result != ZIP_ER_OK) { + zip_error_set(error, result, 0); return false; } - buffer->fragments = fragments; - if ((offsets = realloc(buffer->fragment_offsets, (size_t)offsets_size)) == NULL) { - zip_error_set(error, ZIP_ER_MEMORY, 0); - return false; - } - buffer->fragment_offsets = offsets; - buffer->fragments_capacity = capacity; + + buffer->fragments_capacity = tmp_fragments_capacity; return true; } diff --git a/lib/zip_source_window.c b/lib/zip_source_window.c index 6a58dfff..013e14e2 100644 --- a/lib/zip_source_window.c +++ b/lib/zip_source_window.c @@ -320,15 +320,13 @@ _zip_register_source(zip_t *za, zip_source_t *src) { zip_source_t **open_source; if (za->nopen_source + 1 >= za->nopen_source_alloc) { - unsigned int n; - n = za->nopen_source_alloc + 10; - open_source = (zip_source_t **)realloc(za->open_source, n * sizeof(zip_source_t *)); - if (open_source == NULL) { - zip_error_set(&za->error, ZIP_ER_MEMORY, 0); + int result; + + result = _zip_realloc(&za->open_source, &za->nopen_source_alloc, sizeof(zip_source_t *), 10); + if (result != ZIP_ER_OK) { + zip_error_set(&za->error, result, 0); return -1; } - za->nopen_source_alloc = n; - za->open_source = open_source; } za->open_source[za->nopen_source++] = src; diff --git a/lib/zipint.h b/lib/zipint.h index 92836f13..880c9bb9 100644 --- a/lib/zipint.h +++ b/lib/zipint.h @@ -487,9 +487,12 @@ typedef struct _zip_pkware_keys zip_pkware_keys_t; #endif #endif +#define _zip_realloc(inout_memory, inout_alloced, element_size, additional_elements) (_zip_realloc)((void **)inout_memory, inout_alloced, element_size, additional_elements) +int (_zip_realloc)(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements); zip_int64_t _zip_add_entry(zip_t *); + zip_uint8_t *_zip_buffer_data(zip_buffer_t *buffer); bool _zip_buffer_eof(zip_buffer_t *buffer); void _zip_buffer_free(zip_buffer_t *buffer);