[php-src] Issue #21798: phar: resource leaks on error paths (signature alloc cap, offsetGet temp entry, addFile write error)

[email protected] (iliaal)
Newsgroups php.bugs
Message-ID <b51o4RHuytP8p3da09JDVsSLe5bPahYfm34ACMcLQ7o@main.internal.php.net>
Issue: https://github.com/php/php-src/issues/21798
Author: iliaal

Three low-severity resource management issues in `ext/phar`:

**1. Unbounded `emalloc()` from OpenSSL signature length (`phar.c`)**

`phar_open_from_fp` reads `signature_len` directly from the phar file as a `uint32_t` and passes it to `emalloc()` without an upper bound. A crafted phar can cause a multi-gigabyte allocation on a 64-bit system.

Location: `ext/phar/phar.c` near `PHAR_GET_32(sig_ptr, signature_len); sig = (char *) emalloc(signature_len);`

Fix: cap `signature_len` to a reasonable maximum (e.g. 1 MiB) before the allocation.

**2. `offsetGet` leaks `is_temp_dir` entry on `.phar/*` path rejection (`phar_object.c`)**

`Phar::offsetGet()` calls `phar_get_entry_info_dir()` which may return a temporary directory entry (`is_temp_dir = 1`) that needs to be freed by the caller. Three early-return paths (stub.php, alias.txt, generic .phar prefix) call `RETURN_THROWS()` before reaching the cleanup block.

Location: `PHP_METHOD(Phar, offsetGet)` — the three `RETURN_THROWS()` calls for `.phar/*` paths before the `if (entry->is_temp_dir)` block.

Fix: move the `is_temp_dir` cleanup before the `.phar/*` checks so all exit paths clean up.

**3. `phar_add_file` skips `phar_entry_delref(data)` on short-write error (`phar_object.c`)**

Two `goto finish` calls in the content-write loop exit without calling `phar_entry_delref(data)`. The `finish:` label comes after the delref, so those paths leak the entry reference.

Location: the `if (written_len != contents_len)` and `if (!php_stream_from_zval_no_verify(...))` error branches inside `phar_add_file`.

Fix: add `phar_entry_delref(data)` before each `goto finish` in those branches.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.