Skip to content

Commit df1250d

Browse files
committed
ext/gd: fix out-of-bounds write reading font header on short reads
imageloadfont() read the font header with `(char*)&font[b]`, which scales the byte counter b by sizeof(gdFont) rather than advancing one byte, so a short php_stream_read() (deliverable by a user stream wrapper) makes the loop write hdr_size-b bytes past the emalloc(sizeof(gdFont)) buffer. Index the destination by bytes, matching the body read a few lines below. Closes GH-22380
1 parent 19f595f commit df1250d

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

ext/gd/gd.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ PHP_FUNCTION(imageloadfont)
556556
*/
557557
font = (gdFontPtr) emalloc(sizeof(gdFont));
558558
b = 0;
559-
while (b < hdr_size && (n = php_stream_read(stream, (char*)&font[b], hdr_size - b)) > 0) {
559+
while (b < hdr_size && (n = php_stream_read(stream, (char *) font + b, hdr_size - b)) > 0) {
560560
b += n;
561561
}
562562

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
imageloadfont(): header read must stay in bounds on short reads
3+
--EXTENSIONS--
4+
gd
5+
--FILE--
6+
<?php
7+
/* A user-space wrapper returns one byte per read, so php_stream_read() hands
8+
* imageloadfont()'s header loop a short read on every iteration. The header
9+
* (4 ints) plus a single 1x1 glyph byte form a valid font, which only loads
10+
* when each short read lands at the correct byte offset. */
11+
class drip
12+
{
13+
public $context;
14+
private string $data;
15+
private int $pos = 0;
16+
17+
public function stream_open($path, $mode, $options, &$opened): bool
18+
{
19+
$this->data = pack('V4', 1, 32, 1, 1) . "\x00";
20+
return true;
21+
}
22+
23+
public function stream_read($count): string
24+
{
25+
return $this->pos < strlen($this->data) ? $this->data[$this->pos++] : '';
26+
}
27+
28+
public function stream_eof(): bool
29+
{
30+
return $this->pos >= strlen($this->data);
31+
}
32+
33+
public function stream_stat()
34+
{
35+
return [];
36+
}
37+
38+
public function stream_tell(): int
39+
{
40+
return $this->pos;
41+
}
42+
43+
public function stream_seek($offset, $whence): bool
44+
{
45+
if ($whence === SEEK_CUR) {
46+
$this->pos += $offset;
47+
} elseif ($whence === SEEK_END) {
48+
$this->pos = strlen($this->data) + $offset;
49+
} else {
50+
$this->pos = $offset;
51+
}
52+
return true;
53+
}
54+
55+
public function stream_set_option($option, $arg1, $arg2): bool
56+
{
57+
return false;
58+
}
59+
}
60+
61+
stream_wrapper_register('drip', drip::class);
62+
var_dump(imageloadfont('drip://font') instanceof GdFont);
63+
?>
64+
--EXPECT--
65+
bool(true)

0 commit comments

Comments
 (0)