Fix bzopen() ownership of the stream it wraps

bzopen() with a stream argument, and the wrapper fallback in
_php_stream_bz2open(), pass the stream's own descriptor to
BZ2_bzdopen(). bzlib takes ownership of it and closes it in
BZ2_bzclose(), after which the inner stream close closes the same
descriptor number again. The bz2 stream also keeps a raw pointer to the
inner stream while holding a reference on its resource only, so closing
the inner stream with fclose() left a dangling pointer that the bz2
close dereferenced, while bzlib kept using a descriptor number that
could already belong to another file.

Hand bzlib a duplicate of the descriptor and resolve the inner stream
from its resource on close, so a closed inner stream is skipped.

Closes GH-23824

Author: bukka

NEWS

@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed BcMath\Number results that truncate to zero keeping a negative sign
     and comparing less than zero. (Ilia Alshanetsky)
 
+- BZ2:
+  . Fixed double close of the descriptor and use-after-free of the inner
+    stream when bzopen() is given a stream resource. (Jakub Zelenka)
+
 - CLI
   . Fix GH-22567 (Windows ZTS CLI SAPI should refresh its TSRMLS cache during
     request activation). (matyhtf)

ext/bz2/bz2.c

@@ -59,7 +59,7 @@ ZEND_GET_MODULE(bz2)
 
 struct php_bz2_stream_data_t {
 	BZFILE *bz_file;
-	php_stream *stream;
+	zend_resource *stream_res;
 };
 
 /* {{{ BZip2 stream implementation */
@@ -131,8 +131,15 @@ static int php_bz2iop_close(php_stream *stream, int close_handle)
 		BZ2_bzclose(self->bz_file);
 	}
 
-	if (self->stream) {
-		php_stream_free(self->stream, PHP_STREAM_FREE_CLOSE | (close_handle == 0 ? PHP_STREAM_FREE_PRESERVE_HANDLE : 0));
+	/* The inner stream may have been closed by the user, only its resource is held */
+	if (self->stream_res) {
+		php_stream *inner = zend_fetch_resource2(
+				self->stream_res, NULL, php_file_le_stream(), php_file_le_pstream());
+		if (inner) {
+			php_stream_free(inner, PHP_STREAM_FREE_CLOSE | (close_handle == 0 ? PHP_STREAM_FREE_PRESERVE_HANDLE : 0));
+		} else {
+			zend_list_delete(self->stream_res);
+		}
 	}
 
 	efree(self);
@@ -158,15 +165,33 @@ const php_stream_ops php_stream_bz2io_ops = {
 };
 
 /* {{{ Bzip2 stream openers */
+
+/* bzlib closes the descriptor it is given, so it gets a duplicate */
+static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode)
+{
+	int dup_fd = dup((int) fd);
+	if (dup_fd == -1) {
+		return NULL;
+	}
+
+	BZFILE *bz = BZ2_bzdopen(dup_fd, mode);
+	if (!bz) {
+		close(dup_fd);
+	}
+
+	return bz;
+}
+
 PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz,
 														const char *mode, php_stream *innerstream STREAMS_DC)
 {
 	struct php_bz2_stream_data_t *self;
 
 	self = emalloc(sizeof(*self));
 
-	self->stream = innerstream;
+	self->stream_res = NULL;
 	if (innerstream) {
+		self->stream_res = innerstream->res;
 		GC_ADDREF(innerstream->res);
 	}
 	self->bz_file = bz;
@@ -223,7 +248,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper,
 		if (stream) {
 			php_socket_t fd;
 			if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD, (void **) &fd, REPORT_ERRORS)) {
-				bz_file = BZ2_bzdopen((int)fd, mode);
+				bz_file = php_bz2_bzdopen(fd, mode);
 			}
 		}
 
@@ -399,7 +424,10 @@ PHP_FUNCTION(bzopen)
 			RETURN_FALSE;
 		}
 
-		bz = BZ2_bzdopen((int)fd, mode);
+		bz = php_bz2_bzdopen(fd, mode);
+		if (!bz) {
+			RETURN_FALSE;
+		}
 
 		stream = php_stream_bz2open_from_BZFILE(bz, mode, stream);
 	} else {

ext/bz2/tests/bzopen_resource_fclose.phpt

@@ -0,0 +1,41 @@
+--TEST--
+bzopen() on a stream resource: the inner stream may be closed first
+--EXTENSIONS--
+bz2
+--FILE--
+<?php
+$file = tempnam(sys_get_temp_dir(), 'bz2');
+$victim = tempnam(sys_get_temp_dir(), 'bz2');
+$payload = str_repeat("payload", 100);
+
+$fp = fopen($file, 'w');
+$bz = bzopen($fp, 'w');
+var_dump(bzwrite($bz, $payload));
+
+// The bz2 stream owns its own descriptor, so closing the inner stream neither
+// invalidates it nor makes it write into the next descriptor opened
+var_dump(fclose($fp));
+$other = fopen($victim, 'w');
+bzclose($bz);
+var_dump(fclose($other));
+
+var_dump(bzdecompress(file_get_contents($file)) === $payload);
+var_dump(filesize($victim));
+
+$fp = fopen($file, 'r');
+$bz = bzopen($fp, 'r');
+var_dump(fclose($fp));
+var_dump(bzread($bz, 8192) === $payload);
+bzclose($bz);
+
+unlink($file);
+unlink($victim);
+?>
+--EXPECT--
+int(700)
+bool(true)
+bool(true)
+bool(true)
+int(0)
+bool(true)
+bool(true)