Skip to content

Commit ae506de

Browse files
committed
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.
1 parent 39f98f7 commit ae506de

2 files changed

Lines changed: 76 additions & 6 deletions

File tree

‎ext/bz2/bz2.c‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ ZEND_GET_MODULE(bz2)
5757

5858
struct php_bz2_stream_data_t {
5959
BZFILE *bz_file;
60-
php_stream *stream;
60+
zend_resource *stream_res;
6161
};
6262

6363
/* {{{ BZip2 stream implementation */
@@ -129,8 +129,15 @@ static int php_bz2iop_close(php_stream *stream, int close_handle)
129129
BZ2_bzclose(self->bz_file);
130130
}
131131

132-
if (self->stream) {
133-
php_stream_free(self->stream, PHP_STREAM_FREE_CLOSE | (close_handle == 0 ? PHP_STREAM_FREE_PRESERVE_HANDLE : 0));
132+
/* The inner stream may have been closed by the user, only its resource is held */
133+
if (self->stream_res) {
134+
php_stream *inner = zend_fetch_resource2(
135+
self->stream_res, NULL, php_file_le_stream(), php_file_le_pstream());
136+
if (inner) {
137+
php_stream_free(inner, PHP_STREAM_FREE_CLOSE | (close_handle == 0 ? PHP_STREAM_FREE_PRESERVE_HANDLE : 0));
138+
} else {
139+
zend_list_delete(self->stream_res);
140+
}
134141
}
135142

136143
efree(self);
@@ -156,15 +163,33 @@ const php_stream_ops php_stream_bz2io_ops = {
156163
};
157164

158165
/* {{{ Bzip2 stream openers */
166+
167+
/* bzlib closes the descriptor it is given, so it gets a duplicate */
168+
static BZFILE *php_bz2_bzdopen(php_socket_t fd, const char *mode)
169+
{
170+
int dup_fd = dup((int) fd);
171+
if (dup_fd == -1) {
172+
return NULL;
173+
}
174+
175+
BZFILE *bz = BZ2_bzdopen(dup_fd, mode);
176+
if (!bz) {
177+
close(dup_fd);
178+
}
179+
180+
return bz;
181+
}
182+
159183
PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz,
160184
const char *mode, php_stream *innerstream STREAMS_DC)
161185
{
162186
struct php_bz2_stream_data_t *self;
163187

164188
self = emalloc(sizeof(*self));
165189

166-
self->stream = innerstream;
190+
self->stream_res = NULL;
167191
if (innerstream) {
192+
self->stream_res = innerstream->res;
168193
GC_ADDREF(innerstream->res);
169194
}
170195
self->bz_file = bz;
@@ -221,7 +246,7 @@ PHP_BZ2_API php_stream *_php_stream_bz2open(php_stream_wrapper *wrapper,
221246
if (stream) {
222247
php_socket_t fd;
223248
if (SUCCESS == php_stream_cast(stream, PHP_STREAM_AS_FD, (void **) &fd, REPORT_ERRORS)) {
224-
bz_file = BZ2_bzdopen((int)fd, mode);
249+
bz_file = php_bz2_bzdopen(fd, mode);
225250
}
226251
}
227252

@@ -407,7 +432,10 @@ PHP_FUNCTION(bzopen)
407432
RETURN_FALSE;
408433
}
409434

410-
bz = BZ2_bzdopen((int)fd, mode);
435+
bz = php_bz2_bzdopen(fd, mode);
436+
if (!bz) {
437+
RETURN_FALSE;
438+
}
411439

412440
stream = php_stream_bz2open_from_BZFILE(bz, mode, stream);
413441
} else {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
bzopen() on a stream resource: the inner stream may be closed first
3+
--EXTENSIONS--
4+
bz2
5+
--FILE--
6+
<?php
7+
$file = tempnam(sys_get_temp_dir(), 'bz2');
8+
$victim = tempnam(sys_get_temp_dir(), 'bz2');
9+
$payload = str_repeat("payload", 100);
10+
11+
$fp = fopen($file, 'w');
12+
$bz = bzopen($fp, 'w');
13+
var_dump(bzwrite($bz, $payload));
14+
15+
// The bz2 stream owns its own descriptor, so closing the inner stream neither
16+
// invalidates it nor makes it write into the next descriptor opened
17+
var_dump(fclose($fp));
18+
$other = fopen($victim, 'w');
19+
var_dump(bzclose($bz));
20+
var_dump(fclose($other));
21+
22+
var_dump(bzdecompress(file_get_contents($file)) === $payload);
23+
var_dump(filesize($victim));
24+
25+
$fp = fopen($file, 'r');
26+
$bz = bzopen($fp, 'r');
27+
var_dump(fclose($fp));
28+
var_dump(bzread($bz, 8192) === $payload);
29+
bzclose($bz);
30+
31+
unlink($file);
32+
unlink($victim);
33+
?>
34+
--EXPECT--
35+
int(700)
36+
bool(true)
37+
bool(false)
38+
bool(true)
39+
bool(true)
40+
int(0)
41+
bool(true)
42+
bool(true)

0 commit comments

Comments
 (0)