Skip to content

Commit 738cdc5

Browse files
ndosschendossche
committed
Fix GH-10834: exif_read_data() cannot read smaller stream wrapper chunk sizes
php_stream_read() may return less than the requested amount of bytes by design. This patch introduces a static function for exif which reads from the stream in a loop until all the requested bytes are read. Test was Co-authored-by: dotpointer
1 parent 0842b13 commit 738cdc5

File tree

2 files changed

+106
-10
lines changed

2 files changed

+106
-10
lines changed

‎ext/exif/exif.c‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,26 @@ zend_module_entry exif_module_entry = {
215215
ZEND_GET_MODULE(exif)
216216
#endif
217217

218+
/* php_stream_read() may return early without reading all data, depending on the chunk size
219+
* and whether it's a URL stream or not. This helper keeps reading until the requested amount
220+
* is read or until there is no more data available to read. */
221+
/* FIXME: perhaps this should be moved to some more general place? */
222+
static ssize_t read_from_stream_looped(php_stream *stream, char *buf, size_t count)
223+
{
224+
ssize_t total_read = 0;
225+
while (total_read < count) {
226+
ssize_t ret = php_stream_read(stream, buf + total_read, count - total_read);
227+
if (ret == -1) {
228+
return -1;
229+
}
230+
if (ret == 0) {
231+
break;
232+
}
233+
total_read += ret;
234+
}
235+
return total_read;
236+
}
237+
218238
/* {{{ php_strnlen
219239
* get length of string if buffer if less than buffer size or buffer size */
220240
static size_t php_strnlen(char* str, size_t maxlen) {
@@ -3311,7 +3331,7 @@ static bool exif_process_IFD_TAG_impl(image_info_type *ImageInfo, char *dir_entr
33113331
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_WARNING, "Wrong file pointer: 0x%08X != 0x%08X", fgot, displacement+offset_val);
33123332
return false;
33133333
}
3314-
fgot = php_stream_read(ImageInfo->infile, value_ptr, byte_count);
3334+
fgot = read_from_stream_looped(ImageInfo->infile, value_ptr, byte_count);
33153335
php_stream_seek(ImageInfo->infile, fpos, SEEK_SET);
33163336
if (fgot != byte_count) {
33173337
EFREE_IF(outside);
@@ -3844,7 +3864,7 @@ static bool exif_scan_JPEG_header(image_info_type *ImageInfo)
38443864
Data[0] = (uchar)lh;
38453865
Data[1] = (uchar)ll;
38463866

3847-
got = php_stream_read(ImageInfo->infile, (char*)(Data+2), itemlen-2); /* Read the whole section. */
3867+
got = read_from_stream_looped(ImageInfo->infile, (char*)(Data+2), itemlen-2); /* Read the whole section. */
38483868
if (got != itemlen-2) {
38493869
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_WARNING, "Error reading from file: got=x%04X(=%d) != itemlen-2=x%04X(=%d)", got, got, itemlen-2, itemlen-2);
38503870
return false;
@@ -3862,7 +3882,7 @@ static bool exif_scan_JPEG_header(image_info_type *ImageInfo)
38623882
size = ImageInfo->FileSize - fpos;
38633883
sn = exif_file_sections_add(ImageInfo, M_PSEUDO, size, NULL);
38643884
Data = ImageInfo->file.list[sn].data;
3865-
got = php_stream_read(ImageInfo->infile, (char*)Data, size);
3885+
got = read_from_stream_looped(ImageInfo->infile, (char*)Data, size);
38663886
if (got != size) {
38673887
EXIF_ERRLOG_FILEEOF(ImageInfo)
38683888
return false;
@@ -4039,7 +4059,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
40394059
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Read from TIFF: filesize(x%04X), IFD dir(x%04X + x%04X)", ImageInfo->FileSize, dir_offset, 2);
40404060
#endif
40414061
php_stream_seek(ImageInfo->infile, dir_offset, SEEK_SET); /* we do not know the order of sections */
4042-
php_stream_read(ImageInfo->infile, (char*)ImageInfo->file.list[sn].data, 2);
4062+
read_from_stream_looped(ImageInfo->infile, (char*)ImageInfo->file.list[sn].data, 2);
40434063
num_entries = php_ifd_get16u(ImageInfo->file.list[sn].data, ImageInfo->motorola_intel);
40444064
dir_size = 2/*num dir entries*/ +12/*length of entry*/*(size_t)num_entries +4/* offset to next ifd (points to thumbnail or NULL)*/;
40454065
if (ImageInfo->FileSize >= dir_size && ImageInfo->FileSize - dir_size >= dir_offset) {
@@ -4049,7 +4069,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
40494069
if (exif_file_sections_realloc(ImageInfo, sn, dir_size)) {
40504070
return false;
40514071
}
4052-
php_stream_read(ImageInfo->infile, (char*)(ImageInfo->file.list[sn].data+2), dir_size-2);
4072+
read_from_stream_looped(ImageInfo->infile, (char*)(ImageInfo->file.list[sn].data+2), dir_size-2);
40534073
next_offset = php_ifd_get32u(ImageInfo->file.list[sn].data + dir_size - 4, ImageInfo->motorola_intel);
40544074
#ifdef EXIF_DEBUG
40554075
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Read from TIFF done, next offset x%04X", next_offset);
@@ -4137,7 +4157,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
41374157
#ifdef EXIF_DEBUG
41384158
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Read from TIFF: filesize(x%04X), IFD(x%04X + x%04X)", ImageInfo->FileSize, dir_offset, ifd_size);
41394159
#endif
4140-
php_stream_read(ImageInfo->infile, (char*)(ImageInfo->file.list[sn].data+dir_size), ifd_size-dir_size);
4160+
read_from_stream_looped(ImageInfo->infile, (char*)(ImageInfo->file.list[sn].data+dir_size), ifd_size-dir_size);
41414161
#ifdef EXIF_DEBUG
41424162
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_NOTICE, "Read from TIFF, done");
41434163
#endif
@@ -4188,7 +4208,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
41884208
if (!ImageInfo->Thumbnail.data) {
41894209
ImageInfo->Thumbnail.data = safe_emalloc(ImageInfo->Thumbnail.size, 1, 0);
41904210
php_stream_seek(ImageInfo->infile, ImageInfo->Thumbnail.offset, SEEK_SET);
4191-
fgot = php_stream_read(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
4211+
fgot = read_from_stream_looped(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
41924212
if (fgot != ImageInfo->Thumbnail.size) {
41934213
EXIF_ERRLOG_THUMBEOF(ImageInfo)
41944214
efree(ImageInfo->Thumbnail.data);
@@ -4228,7 +4248,7 @@ static bool exif_process_IFD_in_TIFF_impl(image_info_type *ImageInfo, size_t dir
42284248
if (!ImageInfo->Thumbnail.data && ImageInfo->Thumbnail.offset && ImageInfo->Thumbnail.size && ImageInfo->read_thumbnail) {
42294249
ImageInfo->Thumbnail.data = safe_emalloc(ImageInfo->Thumbnail.size, 1, 0);
42304250
php_stream_seek(ImageInfo->infile, ImageInfo->Thumbnail.offset, SEEK_SET);
4231-
fgot = php_stream_read(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
4251+
fgot = read_from_stream_looped(ImageInfo->infile, ImageInfo->Thumbnail.data, ImageInfo->Thumbnail.size);
42324252
if (fgot != ImageInfo->Thumbnail.size) {
42334253
EXIF_ERRLOG_THUMBEOF(ImageInfo)
42344254
efree(ImageInfo->Thumbnail.data);
@@ -4283,7 +4303,7 @@ static bool exif_scan_FILE_header(image_info_type *ImageInfo)
42834303

42844304
if (ImageInfo->FileSize >= 2) {
42854305
php_stream_seek(ImageInfo->infile, 0, SEEK_SET);
4286-
if (php_stream_read(ImageInfo->infile, (char*)file_header, 2) != 2) {
4306+
if (read_from_stream_looped(ImageInfo->infile, (char*)file_header, 2) != 2) {
42874307
return false;
42884308
}
42894309
if ((file_header[0]==0xff) && (file_header[1]==M_SOI)) {
@@ -4294,7 +4314,7 @@ static bool exif_scan_FILE_header(image_info_type *ImageInfo)
42944314
exif_error_docref(NULL EXIFERR_CC, ImageInfo, E_WARNING, "Invalid JPEG file");
42954315
}
42964316
} else if (ImageInfo->FileSize >= 8) {
4297-
if (php_stream_read(ImageInfo->infile, (char*)(file_header+2), 6) != 6) {
4317+
if (read_from_stream_looped(ImageInfo->infile, (char*)(file_header+2), 6) != 6) {
42984318
return false;
42994319
}
43004320
if (!memcmp(file_header, "II\x2A\x00", 4)) {

‎ext/exif/tests/gh10834.phpt‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
--TEST--
2+
GH-10834 (exif_read_data() cannot read smaller stream wrapper chunk sizes)
3+
--FILE--
4+
<?php
5+
class VariableStream {
6+
private $data;
7+
private $position;
8+
9+
function stream_close() {
10+
return true;
11+
}
12+
13+
function stream_eof() {
14+
return $this->position >= strlen($this->data);
15+
}
16+
17+
function stream_open($path, $mode, $options, &$opened_path) {
18+
$this->position = 0;
19+
$this->data = file_get_contents(__DIR__.'/bug50845.jpg');
20+
return true;
21+
}
22+
23+
function stream_seek($offset, $whence) {
24+
switch ($whence) {
25+
case SEEK_SET:
26+
if ($offset < strlen($this->data) && $offset >= 0) {
27+
$this->position = $offset;
28+
return true;
29+
} else {
30+
return false;
31+
}
32+
break;
33+
case SEEK_CUR:
34+
if ($offset >= 0) {
35+
$this->position += $offset;
36+
return true;
37+
} else {
38+
return false;
39+
}
40+
break;
41+
case SEEK_END:
42+
if (strlen($this->data) + $offset >= 0) {
43+
$this->position = strlen($this->data) + $offset;
44+
return true;
45+
} else {
46+
return false;
47+
}
48+
break;
49+
default:
50+
return false;
51+
}
52+
}
53+
54+
function stream_read($count) {
55+
$ret = substr($this->data, $this->position, $count);
56+
$this->position += strlen($ret);
57+
return $ret;
58+
}
59+
60+
function stream_tell() {
61+
return $this->position;
62+
}
63+
}
64+
65+
stream_wrapper_register('var', 'VariableStream');
66+
67+
$fp = fopen('var://myvar', 'rb');
68+
69+
stream_set_chunk_size($fp, 10);
70+
$headers = exif_read_data($fp);
71+
var_dump(is_array($headers));
72+
73+
fclose($fp);
74+
?>
75+
--EXPECT--
76+
bool(true)

0 commit comments

Comments
 (0)