Skip to content

Commit 3718566

Browse files
authored
Merge pull request #86 from kjdev/fix/heap-use-after-free
fix: heap-use-after-free in php_brotli_decompress_read
2 parents 415c2bd + 0efb752 commit 3718566

2 files changed

Lines changed: 54 additions & 22 deletions

File tree

brotli.c

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,7 @@ typedef struct _php_brotli_stream_data {
797797
php_brotli_context ctx;
798798
BrotliDecoderResult result;
799799
php_stream *stream;
800+
uint8_t *input_buf;
800801
} php_brotli_stream_data;
801802

802803
#define STREAM_DATA_FROM_STREAM() \
@@ -822,6 +823,10 @@ static int php_brotli_decompress_close(php_stream *stream,
822823

823824
php_brotli_context_close(&self->ctx);
824825

826+
if (self->input_buf) {
827+
efree(self->input_buf);
828+
}
829+
825830
efree(self);
826831

827832
stream->abstract = NULL;
@@ -845,48 +850,33 @@ static ssize_t php_brotli_decompress_read(php_stream *stream,
845850
STREAM_DATA_FROM_STREAM();
846851

847852
/* input */
848-
uint8_t *input = (uint8_t *)emalloc(PHP_BROTLI_BUFFER_SIZE);
849-
if (!input) {
850-
#if PHP_VERSION_ID < 70400
851-
return 0;
852-
#else
853-
return -1;
854-
#endif
853+
if (!self->input_buf) {
854+
self->input_buf = emalloc(PHP_BROTLI_BUFFER_SIZE);
855855
}
856856

857857
if (self->result == BROTLI_DECODER_RESULT_NEEDS_MORE_INPUT) {
858858
if (php_stream_eof(self->stream)) {
859859
/* corrupt input */
860-
efree(input);
861860
#if PHP_VERSION_ID < 70400
862861
return 0;
863862
#else
864863
return -1;
865864
#endif
866865
}
867-
self->ctx.available_in = php_stream_read(self->stream, input,
866+
self->ctx.available_in = php_stream_read(self->stream, self->input_buf,
868867
PHP_BROTLI_BUFFER_SIZE);
869868
if (!self->ctx.available_in) {
870-
efree(input);
871869
#if PHP_VERSION_ID < 70400
872870
return 0;
873871
#else
874872
return -1;
875873
#endif
876874
}
877-
self->ctx.next_in = input;
875+
self->ctx.next_in = self->input_buf;
878876
}
879877

880878
/* output */
881879
uint8_t *output = (uint8_t *)emalloc(count);
882-
if (!output) {
883-
efree(input);
884-
#if PHP_VERSION_ID < 70400
885-
return 0;
886-
#else
887-
return -1;
888-
#endif
889-
}
890880
self->ctx.available_out = count;
891881
self->ctx.next_out = output;
892882

@@ -912,15 +902,15 @@ static ssize_t php_brotli_decompress_read(php_stream *stream,
912902
break;
913903
}
914904
self->ctx.available_in = php_stream_read(self->stream,
915-
input, count);
916-
self->ctx.next_in = input;
905+
self->input_buf,
906+
PHP_BROTLI_BUFFER_SIZE);
907+
self->ctx.next_in = self->input_buf;
917908
} else {
918909
/* decoder error */
919910
break;
920911
}
921912
}
922913

923-
efree(input);
924914
efree(output);
925915

926916
return ret;

tests/streams_007.phpt

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
compress.brotli streams read with chunk size exceeding internal buffer
3+
--FILE--
4+
<?php
5+
$file = dirname(__FILE__) . '/streams_007.out';
6+
7+
// PHP_BROTLI_BUFFER_SIZE is 1 << 19 (524288). Use a payload that does not
8+
// compress (so the on-disk size also exceeds the internal buffer) and a read
9+
// chunk larger than that, exercising the NEEDS_MORE_INPUT path with
10+
// count > PHP_BROTLI_BUFFER_SIZE.
11+
$chunk = 1024 * 1024; // 1 MiB > 524288
12+
$data = random_bytes(1024 * 1024); // uncompressible payload
13+
var_dump(strlen($data) > 524288);
14+
15+
echo "Compress\n";
16+
$dst = fopen('compress.brotli://' . $file, 'wb');
17+
var_dump(fwrite($dst, $data) === strlen($data));
18+
var_dump(fclose($dst));
19+
var_dump(filesize($file) > 524288);
20+
21+
echo "Decompress\n";
22+
$fp = fopen('compress.brotli://' . $file, 'rb');
23+
stream_set_chunk_size($fp, $chunk);
24+
$out = stream_get_contents($fp);
25+
var_dump(fclose($fp));
26+
var_dump(strlen($out) === strlen($data));
27+
var_dump($out === $data);
28+
29+
@unlink($file);
30+
?>
31+
===DONE===
32+
--EXPECTF--
33+
bool(true)
34+
Compress
35+
bool(true)
36+
bool(true)
37+
bool(true)
38+
Decompress
39+
bool(true)
40+
bool(true)
41+
bool(true)
42+
===DONE===

0 commit comments

Comments
 (0)