]> granicus.if.org Git - php/commitdiff
Fix use-after-free in stream freeing during shutdown
authorNikita Popov <nikita.ppv@gmail.com>
Wed, 26 Jun 2019 08:58:29 +0000 (10:58 +0200)
committerNikita Popov <nikita.ppv@gmail.com>
Thu, 27 Jun 2019 07:45:23 +0000 (09:45 +0200)
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!)

ext/mysqlnd/mysqlnd_vio.c
main/streams/cast.c
main/streams/streams.c

index 5dd4d0657162af99c5c5a6e1aebbcbd12bc318e0..99c41f0fe798e9844341e024946479d539dc7622 100644 (file)
@@ -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);
        }
index 59832c54a86327146a2f4717f0440a6b8159a11f..262ec5e38f1900ad4813918e1494808b7bbb0138 100644 (file)
@@ -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) */
 
index 08b8e6201426a6b9dee85cb8da3739514b2774d8..5f467c12cefbb84fedf1f606839204ff43e0b95e 100644 (file)
@@ -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;
        }