Skip to content

Commit 69166c3

Browse files
ndosschendossche
committed
Merge branch 'PHP-8.5'
* PHP-8.5: Fix GH-20582: Heap Buffer Overflow in iptcembed
2 parents 004934c + 4ed8fce commit 69166c3

File tree

2 files changed

+104
-28
lines changed

2 files changed

+104
-28
lines changed

‎ext/standard/iptc.c‎

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,24 @@
7373
#define M_APP15 0xef
7474

7575
/* {{{ php_iptc_put1 */
76-
static int php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf)
76+
static int php_iptc_put1(FILE *fp, int spool, unsigned char c, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
7777
{
7878
if (spool > 0)
7979
PUTC(c);
8080

81-
if (spoolbuf) *(*spoolbuf)++ = c;
81+
if (spoolbuf) {
82+
if (UNEXPECTED(*spoolbuf >= spoolbuf_end)) {
83+
return EOF;
84+
}
85+
*(*spoolbuf)++ = c;
86+
}
8287

8388
return c;
8489
}
8590
/* }}} */
8691

8792
/* {{{ php_iptc_get1 */
88-
static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf)
93+
static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
8994
{
9095
int c;
9196
char cc;
@@ -99,66 +104,71 @@ static int php_iptc_get1(FILE *fp, int spool, unsigned char **spoolbuf)
99104
PUTC(cc);
100105
}
101106

102-
if (spoolbuf) *(*spoolbuf)++ = c;
107+
if (spoolbuf) {
108+
if (UNEXPECTED(*spoolbuf >= spoolbuf_end)) {
109+
return EOF;
110+
}
111+
*(*spoolbuf)++ = c;
112+
}
103113

104114
return c;
105115
}
106116
/* }}} */
107117

108118
/* {{{ php_iptc_read_remaining */
109-
static int php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf)
119+
static int php_iptc_read_remaining(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
110120
{
111-
while (php_iptc_get1(fp, spool, spoolbuf) != EOF) continue;
121+
while (php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end) != EOF) continue;
112122

113123
return M_EOI;
114124
}
115125
/* }}} */
116126

117127
/* {{{ php_iptc_skip_variable */
118-
static int php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf)
128+
static int php_iptc_skip_variable(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
119129
{
120130
unsigned int length;
121131
int c1, c2;
122132

123-
if ((c1 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
133+
if ((c1 = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF) return M_EOI;
124134

125-
if ((c2 = php_iptc_get1(fp, spool, spoolbuf)) == EOF) return M_EOI;
135+
if ((c2 = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF) return M_EOI;
126136

127137
length = (((unsigned char) c1) << 8) + ((unsigned char) c2);
128138

129139
length -= 2;
130140

131141
while (length--)
132-
if (php_iptc_get1(fp, spool, spoolbuf) == EOF) return M_EOI;
142+
if (php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end) == EOF) return M_EOI;
133143

134144
return 0;
135145
}
136146
/* }}} */
137147

138148
/* {{{ php_iptc_next_marker */
139-
static int php_iptc_next_marker(FILE *fp, int spool, unsigned char **spoolbuf)
149+
static int php_iptc_next_marker(FILE *fp, int spool, unsigned char **spoolbuf, const unsigned char *spoolbuf_end)
140150
{
141151
int c;
142152

143153
/* skip unimportant stuff */
144154

145-
c = php_iptc_get1(fp, spool, spoolbuf);
155+
c = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end);
146156

147157
if (c == EOF) return M_EOI;
148158

149159
while (c != 0xff) {
150-
if ((c = php_iptc_get1(fp, spool, spoolbuf)) == EOF)
160+
if ((c = php_iptc_get1(fp, spool, spoolbuf, spoolbuf_end)) == EOF)
151161
return M_EOI; /* we hit EOF */
152162
}
153163

154164
/* get marker byte, swallowing possible padding */
155165
do {
156-
c = php_iptc_get1(fp, 0, 0);
166+
c = php_iptc_get1(fp, 0, 0, NULL);
157167
if (c == EOF)
158168
return M_EOI; /* we hit EOF */
159169
else
160170
if (c == 0xff)
161-
php_iptc_put1(fp, spool, (unsigned char)c, spoolbuf);
171+
php_iptc_put1(fp, spool, (unsigned char)c, spoolbuf, spoolbuf_end);
162172
} while (c == 0xff);
163173

164174
return (unsigned int) c;
@@ -178,6 +188,7 @@ PHP_FUNCTION(iptcembed)
178188
size_t inx;
179189
zend_string *spoolbuf = NULL;
180190
unsigned char *poi = NULL;
191+
unsigned char *spoolbuf_end = NULL;
181192
zend_stat_t sb = {0};
182193
bool written = 0;
183194

@@ -210,18 +221,20 @@ PHP_FUNCTION(iptcembed)
210221

211222
spoolbuf = zend_string_safe_alloc(1, iptcdata_len + sizeof(psheader) + 1024 + 1, sb.st_size, 0);
212223
poi = (unsigned char*)ZSTR_VAL(spoolbuf);
224+
spoolbuf_end = poi + ZSTR_LEN(spoolbuf);
213225
memset(poi, 0, iptcdata_len + sizeof(psheader) + sb.st_size + 1024 + 1);
214226
}
215227

216-
if (php_iptc_get1(fp, spool, poi?&poi:0) != 0xFF) {
228+
if (php_iptc_get1(fp, spool, poi?&poi:0, spoolbuf_end) != 0xFF) {
217229
fclose(fp);
218230
if (spoolbuf) {
219231
zend_string_efree(spoolbuf);
220232
}
221233
RETURN_FALSE;
222234
}
223235

224-
if (php_iptc_get1(fp, spool, poi?&poi:0) != 0xD8) {
236+
if (php_iptc_get1(fp, spool, poi?&poi:0, spoolbuf_end) != 0xD8) {
237+
err:
225238
fclose(fp);
226239
if (spoolbuf) {
227240
zend_string_efree(spoolbuf);
@@ -230,20 +243,22 @@ PHP_FUNCTION(iptcembed)
230243
}
231244

232245
while (!done) {
233-
marker = php_iptc_next_marker(fp, spool, poi?&poi:0);
246+
marker = php_iptc_next_marker(fp, spool, poi?&poi:0, spoolbuf_end);
234247

235248
if (marker == M_EOI) { /* EOF */
236249
break;
237250
} else if (marker != M_APP13) {
238-
php_iptc_put1(fp, spool, (unsigned char)marker, poi?&poi:0);
251+
if (php_iptc_put1(fp, spool, (unsigned char)marker, poi?&poi:0, spoolbuf_end) < 0) {
252+
goto err;
253+
}
239254
}
240255

241256
switch (marker) {
242257
case M_APP13:
243258
/* we are going to write a new APP13 marker, so don't output the old one */
244-
php_iptc_skip_variable(fp, 0, 0);
259+
php_iptc_skip_variable(fp, 0, 0, spoolbuf_end);
245260
fgetc(fp); /* skip already copied 0xFF byte */
246-
php_iptc_read_remaining(fp, spool, poi?&poi:0);
261+
php_iptc_read_remaining(fp, spool, poi?&poi:0, spoolbuf_end);
247262
done = 1;
248263
break;
249264

@@ -256,7 +271,7 @@ PHP_FUNCTION(iptcembed)
256271
}
257272
written = 1;
258273

259-
php_iptc_skip_variable(fp, spool, poi?&poi:0);
274+
php_iptc_skip_variable(fp, spool, poi?&poi:0, spoolbuf_end);
260275

261276
if (iptcdata_len & 1) {
262277
iptcdata_len++; /* make the length even */
@@ -266,32 +281,41 @@ PHP_FUNCTION(iptcembed)
266281
psheader[ 3 ] = (iptcdata_len+28)&0xff;
267282

268283
for (inx = 0; inx < 28; inx++) {
269-
php_iptc_put1(fp, spool, psheader[inx], poi?&poi:0);
284+
if (php_iptc_put1(fp, spool, psheader[inx], poi?&poi:0, spoolbuf_end) < 0) {
285+
goto err;
286+
}
270287
}
271288

272-
php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len>>8), poi?&poi:0);
273-
php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len&0xff), poi?&poi:0);
289+
if (php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len>>8), poi?&poi:0, spoolbuf_end) < 0) {
290+
goto err;
291+
}
292+
if (php_iptc_put1(fp, spool, (unsigned char)(iptcdata_len&0xff), poi?&poi:0, spoolbuf_end) < 0) {
293+
goto err;
294+
}
274295

275296
for (inx = 0; inx < iptcdata_len; inx++) {
276-
php_iptc_put1(fp, spool, iptcdata[inx], poi?&poi:0);
297+
if (php_iptc_put1(fp, spool, iptcdata[inx], poi?&poi:0, spoolbuf_end) < 0) {
298+
goto err;
299+
}
277300
}
278301
break;
279302

280303
case M_SOS:
281304
/* we hit data, no more marker-inserting can be done! */
282-
php_iptc_read_remaining(fp, spool, poi?&poi:0);
305+
php_iptc_read_remaining(fp, spool, poi?&poi:0, spoolbuf_end);
283306
done = 1;
284307
break;
285308

286309
default:
287-
php_iptc_skip_variable(fp, spool, poi?&poi:0);
310+
php_iptc_skip_variable(fp, spool, poi?&poi:0, spoolbuf_end);
288311
break;
289312
}
290313
}
291314

292315
fclose(fp);
293316

294317
if (spool < 2) {
318+
*poi = '\0';
295319
spoolbuf = zend_string_truncate(spoolbuf, poi - (unsigned char*)ZSTR_VAL(spoolbuf), 0);
296320
RETURN_NEW_STR(spoolbuf);
297321
} else {

‎ext/standard/tests/image/gh20582.phpt‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
GH-20582 (Heap Buffer Overflow in iptcembed)
3+
--CREDITS--
4+
Nikita Sveshnikov (Positive Technologies)
5+
ndossche
6+
--SKIPIF--
7+
<?php
8+
if (PHP_OS_FAMILY === "Windows") die("skip Only for platforms with FIFO pipes");
9+
?>
10+
--FILE--
11+
<?php
12+
13+
$pipe = __DIR__.'/gh20582.pipe.jpg';
14+
15+
// Create named pipe (FIFO)
16+
if (!file_exists($pipe)) {
17+
if (!posix_mkfifo($pipe, 0666)) {
18+
die("Failed to create FIFO\n");
19+
}
20+
}
21+
22+
$descriptorspec = array(
23+
0 => STDIN,
24+
1 => STDOUT,
25+
2 => STDOUT,
26+
);
27+
$pipes = [];
28+
$proc = proc_open([PHP_BINARY, '-n', '-r', "var_dump(iptcembed('A', '$pipe'));"], $descriptorspec, $pipes);
29+
30+
// Blocks until a reader opens it
31+
$fp = fopen($pipe, 'wb') or die("Failed to open FIFO");
32+
33+
// Write header
34+
$data = "\xFF\xD8"; // SOI marker
35+
$data .= "\xFF\xE0\x00\x10"; // APP0 marker (JFIF)
36+
$data .= "JFIF" . str_repeat("\x00", 9);
37+
$data .= "\xFF\xDA\x00\x08"; // SOS marker
38+
$data .= str_repeat("\x00", 6);
39+
fwrite($fp, $data);
40+
41+
// Write garbage
42+
fwrite($fp, str_repeat("A", 5120));
43+
44+
fclose($fp);
45+
46+
?>
47+
--CLEAN--
48+
<?php
49+
@unlink(__DIR__.'/gh20582.pipe.jpg');
50+
?>
51+
--EXPECTF--
52+
string(1055) "ÿØÿà%0JFIF%0%0%0%0%0%0%0%0%0ÿÿí%0Photoshop 3.0%08BIM%0%0%0%0%0A%0Ú%0%0%0%0%0%0%0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"

0 commit comments

Comments
 (0)