Skip to content

zip_close: check for allocation overflow and set error on failure - #588

Closed
filtede98 wants to merge 1 commit into
nih-at:mainfrom
filtede98:fix-zip-close-allocation-safety
Closed

filtede98 wants to merge 1 commit into
nih-at:mainfrom
filtede98:fix-zip-close-allocation-safety

Conversation

@filtede98

Copy link
Copy Markdown
Contributor

In zip_close(), survivors is a 64-bit integer (zip_uint64_t).

  1. Integer overflow prevention: On 32-bit platforms, the allocation size calculation sizeof(filelist[0]) * (size_t)survivors can wrap around size_t if survivors > SIZE_MAX / sizeof(filelist[0]). This would result in an undersized buffer allocation and subsequent out-of-bounds writes when populating ilelist[j]. We add an explicit bounds check before multiplying.
  2. Missing error code on allocation failure: If malloc() failed, zip_close() previously returned -1 without setting za->error via zip_error_set(&za->error, ZIP_ER_MEMORY, 0). Callers checking zip_get_error(za) / zip_strerror(za) received a stale or uninitialized error code.
  3. Empty archive handling: When survivors == 0 (e.g. keeping an empty archive with ZIP_AFL_CREATE_OR_KEEP_FILE_FOR_EMPTY_ARCHIVE), we set ilelist = NULL directly rather than invoking malloc(0), which can return NULL on some platforms/allocators and cause spurious failure.
  4. qsort guard: qsort() is guarded with && survivors > 0 to avoid passing a NULL buffer pointer.

In zip_close(), survivors is a 64-bit integer (zip_uint64_t). On 32-bit platforms, the allocation size calculation sizeof(filelist[0]) * (size_t)survivors could overflow size_t when survivors exceeds SIZE_MAX / sizeof(filelist[0]), leading to an undersized buffer allocation and subsequent out-of-bounds writes.

Additionally:
- When survivors == 0 (e.g. keeping an empty archive), avoid calling malloc(0), which may return NULL on some platforms/allocators and cause spurious failure.
- Ensure zip_error_set(&za->error, ZIP_ER_MEMORY, 0) is called if allocation fails so callers receive a consistent error code.
- Guard the qsort() call to avoid passing a NULL pointer when survivors == 0.
@dillof dillof added this to the 1.12 milestone Sep 30, 2026
@dillof dillof added the bug For issues, use type instead. label Sep 30, 2026
dillof added a commit that referenced this pull request Sep 30, 2026
@dillof

dillof commented Sep 30, 2026

Copy link
Copy Markdown
Member

Thanks. The overflow can't happen because we already check that survivors < za->nentry. We applied NULL pointer handling change.

@dillof dillof closed this Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For issues, use type instead.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants