GHSA-jqcx-ccgc-xwhv

Описание

Summary

Buffer mismanagement in phar_dir_read() causes a buffer overflow and a buffer overread later.

Details

https://github.com/php/php-src/blob/be71cadc2f899bc39fe27098042139392e2187db/ext/phar/dirstream.c#L89C1-L116

There are few issues here:

• The if check to_read == 0 || count < ZSTR_LEN(str_key) is not right. In particular, if this is false we know that count >= ZSTR_LEN(str_key). Furthermore, because of to_read = MIN(ZSTR_LEN(str_key), count); we now actually have: to_read == ZSTR_LEN(str_key). If we have a filename length (i.e. ZSTR_LEN(str_key)) equal to sizeof(php_stream_dirent), then we get ZSTR_LEN(str_key) == count == to_read == sizeof(php_stream_dirent).

  Take a look now at this line: ((php_stream_dirent *) buf)->d_name[to_read + 1] = '\0';. Assume sizeof(php_stream_dirent) is 4096 like on most Linuxes. Then we write at offset 4097, which is two bytes outside of buf. This line was actually supposed to use to_read instead of to_read + 1, because now even if it doesn't overflow, it does not properly NUL-terminate the filename. We're just lucky there's a memset above. Correcting that to use to_read doesn't solve the whole problem: it would still overflow because it will write at offset 4096 which is still outside buf.

  This means we have a stack information leak and a buffer write overflow.

• Both the memset and the return value assume that count == sizeof(php_stream_dirent). In principle if there are any callers where that count is smaller, a buffer overflow occurs in the memset. However, inside PHP itself I did not find any such caller, but this may be a concern for third-party extensions.

PoC

I created a reproducer using PHP-8.0 with these configure flags: ./configure --enable-debug --disable-all --enable-phar --with-valgrind using compiler gcc (GCC) 13.1.1 20230429.

Create the malicious Phar file using:

Trigger the buffer overflow and overread using this script:

If you run this in Valgrind, i.e. valgrind ./sapi/cli/php trigger.php you'll find two Valgrind complaints:

The first complaint is because the strlen() function overreads the d_name buffer because it isn't properly NUL-terminated. The second one is because it writes the name using var_dump, and the name contains (uninitialized) stack data. Valgrind won't complain about the stack buffer write overflow, but you can inspect the memory using gdb for example that a piece of the stack gets overwritten.

Impact

Exploiting this is difficult to do and heavily depends on the application you're targetting, but possible in theory I think using a combination of stack info leaks and buffer write overflows. People who inspect contents of untrusted phar files could be affected.