[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.