From: Nikita Popov <nikita.ppv@gmail.com>
Date: Wed, 26 Jun 2019 08:58:29 +0000 (+0200)
Subject: Fix use-after-free in stream freeing during shutdown
X-Git-Tag: php-7.4.0alpha3~167
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6bebe833a2fd65c4d42e3870317a4c6b4c7299ad;p=php

Fix use-after-free in stream freeing during shutdown

Streams will be freed in an unpredictable order during shutdown.
Ignore explicit calls to php_stream_close() entirely to avoid
use-after-free -- instead let the stream resource destructor
deal with it. We have to account for a few special cases:

 * Enclosed streams should be freed, as the resource destructor
   will forward to the enclosing stream.
 * Stream cookies also directly free streams, because we delegate
   to the cookie destruction if one exists.
 * Mysqlnd also directly frees streams, because it explicitly
   removes stream resources (because mysqlnd!)
---

diff --git a/ext/mysqlnd/mysqlnd_vio.c b/ext/mysqlnd/mysqlnd_vio.c
index 5dd4d06571..99c41f0fe7 100644
--- a/ext/mysqlnd/mysqlnd_vio.c
+++ b/ext/mysqlnd/mysqlnd_vio.c
@@ -644,18 +644,17 @@ MYSQLND_METHOD(mysqlnd_vio, close_stream)(MYSQLND_VIO * const net, MYSQLND_STATS
 	if (net && (net_stream = net->data->m.get_stream(net))) {
 		zend_bool pers = net->persistent;
 		DBG_INF_FMT("Freeing stream. abstract=%p", net_stream->abstract);
-		if (pers) {
-			if (EG(active)) {
-				php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE_PERSISTENT | PHP_STREAM_FREE_RSRC_DTOR);
-			} else {
-				/*
-				  otherwise we will crash because the EG(persistent_list) has been freed already,
-				  before the modules are shut down
-				*/
-				php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
-			}
+		/* We removed the resource from the stream, so pass FREE_RSRC_DTOR now to force
+		 * destruction to occur during shutdown, because it won't happen through the resource. */
+		/* TODO: The EG(active) check here is dead -- check IN_SHUTDOWN? */
+		if (pers && EG(active)) {
+			php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE_PERSISTENT | PHP_STREAM_FREE_RSRC_DTOR);
 		} else {
-			php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE);
+			/*
+			  otherwise we will crash because the EG(persistent_list) has been freed already,
+			  before the modules are shut down
+			*/
+			php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
 		}
 		net->data->m.set_stream(net, NULL);
 	}
diff --git a/main/streams/cast.c b/main/streams/cast.c
index 59832c54a8..262ec5e38f 100644
--- a/main/streams/cast.c
+++ b/main/streams/cast.c
@@ -84,7 +84,8 @@ static int stream_cookie_closer(void *cookie)
 
 	/* prevent recursion */
 	stream->fclose_stdiocast = PHP_STREAM_FCLOSE_NONE;
-	return php_stream_free(stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_KEEP_RSRC);
+	return php_stream_free(stream,
+		PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_KEEP_RSRC | PHP_STREAM_FREE_RSRC_DTOR);
 }
 #elif defined(HAVE_FOPENCOOKIE)
 static ssize_t stream_cookie_reader(void *cookie, char *buffer, size_t size)
@@ -126,7 +127,8 @@ static int stream_cookie_closer(void *cookie)
 
 	/* prevent recursion */
 	stream->fclose_stdiocast = PHP_STREAM_FCLOSE_NONE;
-	return php_stream_free(stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_KEEP_RSRC);
+	return php_stream_free(stream,
+		PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_KEEP_RSRC | PHP_STREAM_FREE_RSRC_DTOR);
 }
 #endif /* elif defined(HAVE_FOPENCOOKIE) */
 
diff --git a/main/streams/streams.c b/main/streams/streams.c
index 08b8e62014..5f467c12ce 100644
--- a/main/streams/streams.c
+++ b/main/streams/streams.c
@@ -361,15 +361,22 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options) /* {{{ */
 	int ret = 1;
 	int preserve_handle = close_options & PHP_STREAM_FREE_PRESERVE_HANDLE ? 1 : 0;
 	int release_cast = 1;
-	php_stream_context *context = NULL;
+	php_stream_context *context;
 
-	/* on an resource list destruction, the context, another resource, may have
-	 * already been freed (if it was created after the stream resource), so
-	 * don't reference it */
-	if (EG(active)) {
-		context = PHP_STREAM_CONTEXT(stream);
+	/* During shutdown resources may be released before other resources still holding them.
+	 * When only resoruces are referenced this is not a problem, because they are refcounted
+	 * and will only be fully freed once the refcount drops to zero. However, if php_stream*
+	 * is held directly, we don't have this guarantee. To avoid use-after-free we ignore all
+	 * stream free operations in shutdown unless they come from the resource list destruction,
+	 * or by freeing an enclosed stream (in which case resource list destruction will not have
+	 * freed it). */
+	if ((EG(flags) & EG_FLAGS_IN_SHUTDOWN) &&
+			!(close_options & (PHP_STREAM_FREE_RSRC_DTOR|PHP_STREAM_FREE_IGNORE_ENCLOSING))) {
+		return 1;
 	}
 
+	context = PHP_STREAM_CONTEXT(stream);
+
 	if (stream->flags & PHP_STREAM_FLAG_NO_CLOSE) {
 		preserve_handle = 1;
 	}