Signed/unsigned mix in the EOCD backward scan rejects valid archives with a 65535-byte comment
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
Research direction
Start with mz_zip_reader_locate_header_sig() in src/miniz.h and inspect the backward scan offset calculation around line 5693. The reproducer uses zip_stream_open() from src/zip.h with a valid archive containing a 65535-byte comment. Verify that the scan reaches the EOCD without overflowing and that zip_stream_open() accepts the archive.
Written by the indexing model from the issue text.
Description
A valid ZIP archive whose end-of-central-directory record carries a maximal 65535-byte comment is rejected by zip_stream_open(). The backward scan for the EOCD signature mixes signed and unsigned arithmetic, so the read offset wraps near the start of the buffer.
1. Version tested
master b7c63224ed78596cef8567fe7bfeef045961b8a0 (v0.3.15-6-gb7c6322), current HEAD.
2. Platform / compiler / flags
Linux x86_64, clang 18.1.8, -fsanitize=address,undefined -g -O0.
3. Reproducer (public src/zip.h API)
Generate a valid archive with a maximal comment:
import zipfile
with zipfile.ZipFile("crash_000.zip", "w") as z:
z.writestr("a", b"hello")
z.comment = b"A" * 65535
Open it through the public API:
#include <stdio.h>
#include <stdlib.h>
#include "zip.h"
int main(int argc, char **argv) {
FILE *f = fopen(argv[1], "rb");
fseek(f, 0, SEEK_END); long n = ftell(f); rewind(f);
char *data = malloc(n); fread(data, 1, n, f); fclose(f);
struct zip_t *zip = zip_stream_open(data, (size_t)n, 0, 'r');
printf("zip_stream_open -> %p\n", (void *)zip);
if (zip) zip_stream_close(zip);
free(data);
return 0;
}
git checkout b7c6322
clang -fsanitize=address,undefined -Wno-implicit-function-declaration -g -O0 -c src/zip.c -o zip.o
clang -fsanitize=address,undefined -g -O0 -Isrc poc.c zip.o -o poc
UBSAN_OPTIONS=print_stacktrace=1 ./poc crash_000.zip
The program prints zip_stream_open -> (nil), so the valid archive is rejected.
4. Sanitizer output
src/miniz.h:6133:57: runtime error: addition of unsigned offset to 0x... overflowed to 0x...
#0 in mz_zip_mem_read_func src/miniz.h:6133
#1 in mz_zip_reader_locate_header_sig src/miniz.h:5671
#2 in mz_zip_reader_read_central_dir src/miniz.h:5744
#3 in mz_zip_reader_init_mem src/miniz.h:6162
#4 in zip_stream_openwitherror src/zip.c:3154
#5 in zip_stream_open src/zip.c:3106
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/miniz.h:6133
5. Mechanism and impact
In the vendored miniz, mz_zip_reader_locate_header_sig() computes the next backward read offset as:
// src/miniz.h:5693
cur_file_ofs = MZ_MAX(cur_file_ofs - (sizeof(buf_u32) - 3), 0);
cur_file_ofs is mz_int64 but sizeof(buf_u32) - 3 is size_t, so near the start of the buffer the subtraction is evaluated in unsigned arithmetic and wraps to about 2^64 instead of going negative; MZ_MAX(..., 0) then keeps the wrapped value. The next read in mz_zip_mem_read_func() (src/miniz.h:6133) forms m_pMem + file_ofs with that offset, which UBSan reports as a pointer overflow, and the short read makes mz_zip_reader_read_central_dir() fail, so zip_stream_open() returns NULL.
Impact: a well-formed archive with a maximal 65535-byte comment is rejected, a correctness and denial-of-service issue for any reader that accepts archives with comments. Casting the operand to mz_int64 before the subtraction, so MZ_MAX clamps it to 0, lets the scan reach offset 0 and locate the EOCD.
- Dominant language
- C
- Stars
- 1.6k
- Forks
- 308
- Avg merge
- 14h 22m
- Merged PRs (30d)
- 1
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from kuba--/zip
-
Difficulty 4/5 3-5 days Newbie friendliness 52/100
-
change request enhancement help wanted question under discussion
Difficulty 4/5 3-5 days Newbie friendliness 48/100
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
bug game: 01008BA02525A000 ios
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
area/ysql kind/bug priority/medium status/awaiting-triage
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
yugabyte/yugabyte-db#37523 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 77/100
Maintainers usually reply within 3 days
-
Difficulty 1/5 Under an hour Newbie friendliness 65/100
ly05010419/MacNativeGames#2079 ·
Maintainers usually reply within 1 day