Fix GH-11078: PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors

Although the issue was demonstrated using Curl, the issue is purely in
the streams layer of PHP.

Full analysis is written in GH-11078 [1], but here is the brief version:
Here's what actually happens:
1) We're creating a FILE handle from a stream using the casting mechanism.
   This will create a cookie-based FILE handle using funopen.
2) We're reading stream data using fread from the userspace stream. This will
   temporarily set a buffer into a field _bf.base [2]. This buffer is now equal
   to the upload buffer that Curl allocated and note that that buffer is owned
   by Curl.
3) The fatal error occurs and we bail out from the fread function, notice how
   the reset code is never executed and so the buffer will still point to
   Curl's upload buffer instead of FILE's own buffer [3].
4) The resources are destroyed, this includes our opened stream and because the
   FILE handle is cached, it gets destroyed as well.
   In fact, the stream code calls through fclose on purpose in this case.
5) The fclose code frees the _bs.base buffer [4].
   However, this is not the buffer that FILE owns but the one that Curl owns
   because it isn't reset properly due to the bailout!
6) The objects are getting destroyed, and so the curl free logic is invoked.
   When Curl tries to gracefully clean up, it tries to free the buffer.
   But that buffer is actually already freed mistakingly by the C library!

This also explains why we can't reproduce it on Linux: this bizarre buffer
swapping only happens on macOS and BSD, not on Linux.

To solve this, we switch to an unbuffered mode for cookie-based FILEs.
This avoids any stateful problems related to buffers especially when the
bailout mechanism triggers. As streams have their own buffering
mechanism, I don't expect this to impact performance.

[1] https://github.com/php/php-src/issues/11078#issuecomment-2155616843
[2] 5e566be7a7/stdio/FreeBSD/fread.c (L102-L103)
[3] 5e566be7a7/stdio/FreeBSD/fread.c (L117)
[4] 5e566be7a7/stdio/FreeBSD/fclose.c (L66-L67)

Closes GH-14524.
This commit is contained in:
Niels Dossche 2024-06-09 16:18:11 +02:00
parent 46013f1c55
commit bc558bf7a3
No known key found for this signature in database
GPG Key ID: B8A8AD166DF0E2E5
6 changed files with 88 additions and 2 deletions

4
NEWS
View File

@ -58,6 +58,10 @@ PHP NEWS
. Fixed bug GH-14290 (Member access within null pointer in extension spl).
(nielsdos)
- Streams:
. Fixed bug GH-11078 (PHP Fatal error triggers pointer being freed was not
allocated and malloc: double free for ptr errors). (nielsdos)
06 Jun 2024, PHP 8.2.20
- CGI:

View File

@ -596,6 +596,38 @@ static ZEND_FUNCTION(zend_test_set_fmode)
}
#endif
static ZEND_FUNCTION(zend_test_cast_fread)
{
zval *stream_zv;
php_stream *stream;
FILE *fp;
ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_RESOURCE(stream_zv);
ZEND_PARSE_PARAMETERS_END();
php_stream_from_zval(stream, stream_zv);
if (php_stream_cast(stream, PHP_STREAM_AS_STDIO, (void *) &fp, REPORT_ERRORS) == FAILURE) {
return;
}
size_t size = 10240; /* Must be large enough to trigger the issue */
char *buf = malloc(size);
bool bail = false;
zend_try {
(void) !fread(buf, 1, size, fp);
} zend_catch {
bail = true;
} zend_end_try();
free(buf);
if (bail) {
zend_bailout();
}
}
static zend_object *zend_test_class_new(zend_class_entry *class_type)
{
zend_object *obj = zend_objects_new(class_type);

View File

@ -186,6 +186,9 @@ function zend_test_override_libxml_global_state(): void {}
#if defined(PHP_WIN32)
function zend_test_set_fmode(bool $binary): void {}
#endif
/** @param resource $stream */
function zend_test_cast_fread($stream): void {}
}
namespace ZendTestNS {

View File

@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: b0964f7eabf91dc0fbffdee87257ee4e58dab303 */
* Stub hash: 98cade449e4dcf038166adeee07b17308dd48725 */
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
ZEND_END_ARG_INFO()
@ -118,6 +118,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_set_fmode, 0, 1, IS_VO
ZEND_END_ARG_INFO()
#endif
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_cast_fread, 0, 1, IS_VOID, 0)
ZEND_ARG_INFO(0, stream)
ZEND_END_ARG_INFO()
#define arginfo_ZendTestNS2_namespaced_func arginfo_zend_test_is_pcre_bundled
#define arginfo_ZendTestNS2_namespaced_deprecated_func arginfo_zend_test_void_return
@ -225,6 +229,7 @@ static ZEND_FUNCTION(zend_test_is_pcre_bundled);
#if defined(PHP_WIN32)
static ZEND_FUNCTION(zend_test_set_fmode);
#endif
static ZEND_FUNCTION(zend_test_cast_fread);
static ZEND_FUNCTION(ZendTestNS2_namespaced_func);
static ZEND_FUNCTION(ZendTestNS2_namespaced_deprecated_func);
static ZEND_FUNCTION(ZendTestNS2_ZendSubNS_namespaced_func);
@ -287,6 +292,7 @@ static const zend_function_entry ext_functions[] = {
#if defined(PHP_WIN32)
ZEND_FE(zend_test_set_fmode, arginfo_zend_test_set_fmode)
#endif
ZEND_FE(zend_test_cast_fread, arginfo_zend_test_cast_fread)
ZEND_NS_FALIAS("ZendTestNS2", namespaced_func, ZendTestNS2_namespaced_func, arginfo_ZendTestNS2_namespaced_func)
ZEND_NS_DEP_FALIAS("ZendTestNS2", namespaced_deprecated_func, ZendTestNS2_namespaced_deprecated_func, arginfo_ZendTestNS2_namespaced_deprecated_func)
ZEND_NS_FALIAS("ZendTestNS2", namespaced_aliased_func, zend_test_void_return, arginfo_ZendTestNS2_namespaced_aliased_func)

View File

@ -0,0 +1,34 @@
--TEST--
GH-11078 (PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors)
--EXTENSIONS--
zend_test
--SKIPIF--
<?php
if (getenv('USE_ZEND_ALLOC') === '0') die('skip Zend MM disabled');
if (PHP_OS_FAMILY === 'Windows') die('skip Windows does not support generic stream casting');
?>
--FILE--
<?php
const MEM = 32 * 1024 * 1024;
ini_set('memory_limit', MEM);
class CrashingFifo {
public $context;
function stream_open($path, $mode, $options, &$opened_path): bool {
return true;
}
function stream_read(int $count): false|string|null {
return str_repeat('x', MEM);
}
}
stream_register_wrapper('fifo', CrashingFifo::class);
$readStream = fopen('fifo://1', 'r');
zend_test_cast_fread($readStream);
?>
--EXPECTF--
Fatal error: Allowed memory size of %d bytes exhausted %s

View File

@ -46,7 +46,14 @@ typedef struct {
FILE *fopencookie(void *cookie, const char *mode, COOKIE_IO_FUNCTIONS_T *funcs)
{
return funopen(cookie, funcs->reader, funcs->writer, funcs->seeker, funcs->closer);
FILE *file = funopen(cookie, funcs->reader, funcs->writer, funcs->seeker, funcs->closer);
if (file) {
/* Buffering of FILE handles is stateful.
* A bailout during these can corrupt the state of the FILE handle
* and cause memory corruption errors. See GH-11078. */
setvbuf(file, NULL, _IONBF, 0);
}
return file;
}
# define HAVE_FOPENCOOKIE 1
# define PHP_EMULATE_FOPENCOOKIE 1