Factor our realloc handling to a separate function

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

lib/zip_add_entry.c

@@ -44,32 +44,18 @@ _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;
 
         if (additional_entries < 16) {
             additional_entries = 16;
         }
         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);
+        if (!zip_realloc(&za->entry, &za->nentry_alloc, sizeof(struct zip_entry), additional_entries, &za->error)) {
             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++;

lib/zip_dirent.c

@@ -86,32 +86,21 @@ _zip_cdir_new(zip_uint64_t nentry, zip_error_t *error) {
 
 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;
+    zip_uint64_t i;
 
     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);
-        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);
+    if (!zip_realloc(&cd->entry, &cd->nentry_alloc, sizeof(*(cd->entry)), additional_entries, error)) {
         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;
 }

lib/zip_realloc.c

@@ -0,0 +1,75 @@
+/*
+  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 <[email protected]>
+
+  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 <stdlib.h>
+
+#include "zipint.h"
+
+bool zip_realloc_implementation(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements, zip_error_t *error) {
+    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) {
+        return true;
+    }
+
+    if (old_alloced + additional_elements < old_alloced) {
+        zip_error_set(error, ZIP_ER_MEMORY, 0);
+        return false;
+    }
+
+    new_alloced = old_alloced + additional_elements;
+
+    if (new_alloced > SIZE_MAX / element_size) {
+        zip_error_set(error, ZIP_ER_MEMORY, 0);
+        return false;
+    }
+
+    /* Cast explicitly to size_t to quiet msvc complaints about
+       possible loss of data. At this point we know that the result of
+       "new_alloced * element_size" should fit into size_t. */
+    new_size = (size_t)(new_alloced * element_size);
+
+    if ((new_memory = realloc(old_memory, new_size)) == NULL) {
+        zip_error_set(error, ZIP_ER_MEMORY, 0);
+        return false;
+    }
+
+    *inout_memory = new_memory;
+    *inout_alloced = new_alloced;
+
+    return true;
+}

lib/zip_source_buffer.c

@@ -427,32 +427,31 @@ buffer_free(buffer_t *buffer) {
 
 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;
+    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;
+    if (!zip_realloc(&buffer->fragments, &tmp_fragments_capacity, sizeof(buffer->fragments[0]), additional_elements, error)) {
         return false;
     }
 
-    if ((fragments = realloc(buffer->fragments, (size_t)fragments_size)) == NULL) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
+    /* This is technically not correct if buffer->fragment_capacity is
+       0, but results in the correct required new capacity for
+       buffer->fragment_offsets in all cases (one element more than
+       for buffer->fragments). Also, since we added
+       additional_elements (which is 1 or more) to
+       buffer->fragments_capacity in the previous realloc, we know it
+       can't overflow. */
+    tmp_offsets_capacity = buffer->fragments_capacity + 1;
+    if (!zip_realloc(&buffer->fragment_offsets, &tmp_offsets_capacity, sizeof(buffer->fragment_offsets[0]), additional_elements, error)) {
         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;
 }

lib/zip_source_window.c

@@ -303,7 +303,7 @@ window_read(zip_source_t *src, void *_ctx, void *data, zip_uint64_t len, zip_sou
 
 void
 _zip_deregister_source(zip_t *za, zip_source_t *src) {
-    unsigned int i;
+    zip_uint64_t i;
 
     for (i = 0; i < za->nopen_source; i++) {
         if (za->open_source[i] == src) {
@@ -317,18 +317,10 @@ _zip_deregister_source(zip_t *za, zip_source_t *src) {
 
 int
 _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);
+        if (!zip_realloc(&za->open_source, &za->nopen_source_alloc, sizeof(zip_source_t *), 10, &za->error)) {
             return -1;
         }
-        za->nopen_source_alloc = n;
-        za->open_source = open_source;
     }
 
     za->open_source[za->nopen_source++] = src;

lib/zipint.h

@@ -291,8 +291,8 @@ struct zip {
     zip_uint64_t nentry_alloc; /* number of entries allocated */
     zip_entry_t *entry;        /* entries */
 
-    unsigned int nopen_source;       /* number of open sources using archive */
-    unsigned int nopen_source_alloc; /* number of sources allocated */
+    zip_uint64_t nopen_source;       /* number of open sources using archive */
+    zip_uint64_t nopen_source_alloc; /* number of sources allocated */
     zip_source_t **open_source;      /* open sources using archive */
 
     zip_hash_t *names; /* hash table for name lookup */
@@ -486,6 +486,8 @@ typedef struct _zip_pkware_keys zip_pkware_keys_t;
 #endif
 #endif
 
+#define zip_realloc(inout_memory, inout_alloced, element_size, additional_elements, error) zip_realloc_implementation((void **)inout_memory, inout_alloced, element_size, additional_elements, error)
+bool zip_realloc_implementation(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements, zip_error_t *error);
 
 zip_int64_t _zip_add_entry(zip_t *);