Signed/unsigned mix in the EOCD backward scan rejects valid archives with a 65535-byte comment

Open Beginner friendly
#453 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
backend

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

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from kuba--/zip

All issues in kuba--/zip

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.