Skip to content

Commit 5959a15

Browse files
LamentXU123LamentXU123
committed
Merge branch 'PHP-8.4' into PHP-8.5
* PHP-8.4: ext/phar: Fix ZIP extra field length underflow (#22330)
2 parents d8a5347 + 657f0d6 commit 5959a15

3 files changed

Lines changed: 129 additions & 15 deletions

File tree

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ PHP NEWS
5353
. Fixed a bypass of the magic ".phar" directory protection in
5454
Phar::addEmptyDir() for paths starting with "/.phar", while allowing
5555
non-magic directory names that merely share the ".phar" prefix. (Weilin Du)
56+
. Fixed an integer underflow when parsing ZIP extra fields. (Weilin Du)
5657

5758
- Reflection:
5859
. Preserve class-name case in ReflectionClass::getProperty() error messages

‎ext/phar/tests/zip/zip_extra_underflow.phpt‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
--TEST--
2+
Phar: ZIP extra field length must not underflow
3+
--EXTENSIONS--
4+
phar
5+
--FILE--
6+
<?php
7+
function uint16($value) {
8+
return pack('v', $value);
9+
}
10+
11+
function uint32($value) {
12+
return pack('V', $value);
13+
}
14+
15+
$filename = __DIR__ . '/zip_extra_underflow.zip';
16+
$entry = 'test.txt';
17+
$contents = 'hello';
18+
$crc = crc32($contents);
19+
20+
$local = uint32(0x04034b50)
21+
. uint16(20)
22+
. uint16(0)
23+
. uint16(0)
24+
. uint16(0)
25+
. uint16(0)
26+
. uint32($crc)
27+
. uint32(strlen($contents))
28+
. uint32(strlen($contents))
29+
. uint16(strlen($entry))
30+
. uint16(0)
31+
. $entry
32+
. $contents;
33+
34+
$extra = 'XX' . uint16(1);
35+
36+
/* Old code seeks one byte past the extra field and parses this as another extra header. */
37+
$commentPrefix = 'A'
38+
. 'UT'
39+
. uint16(5)
40+
. "\x01"
41+
. uint32(946684800)
42+
. 'ZZ'
43+
. uint16(65522);
44+
$comment = $commentPrefix . str_repeat('B', 65535 - strlen($commentPrefix));
45+
46+
$central = uint32(0x02014b50)
47+
. uint16(20)
48+
. uint16(20)
49+
. uint16(0)
50+
. uint16(0)
51+
. uint16(0)
52+
. uint16(0)
53+
. uint32($crc)
54+
. uint32(strlen($contents))
55+
. uint32(strlen($contents))
56+
. uint16(strlen($entry))
57+
. uint16(strlen($extra))
58+
. uint16(strlen($comment))
59+
. uint16(0)
60+
. uint16(0)
61+
. uint32(0)
62+
. uint32(0)
63+
. $entry
64+
. $extra
65+
. $comment;
66+
67+
$eocd = uint32(0x06054b50)
68+
. uint16(0)
69+
. uint16(0)
70+
. uint16(1)
71+
. uint16(1)
72+
. uint32(strlen($central))
73+
. uint32(strlen($local))
74+
. uint16(0);
75+
76+
file_put_contents($filename, $local . $central . $eocd);
77+
78+
try {
79+
$phar = new PharData($filename);
80+
echo "Loaded corrupt ZIP\n";
81+
echo $phar[$entry]->getMTime(), "\n";
82+
} catch (Exception $e) {
83+
echo $e->getMessage(), "\n";
84+
}
85+
?>
86+
--CLEAN--
87+
<?php
88+
@unlink(__DIR__ . '/zip_extra_underflow.zip');
89+
?>
90+
--EXPECTF--
91+
phar error: Unable to process extra field header for file in central directory in zip-based phar "%szip_extra_underflow.zip"

‎ext/phar/zip.c‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,30 @@ static inline void phar_write_16(char buffer[2], uint32_t value)
4141
# define PHAR_SET_32(var, value) phar_write_32(var, (uint32_t) (value));
4242
# define PHAR_SET_16(var, value) phar_write_16(var, (uint16_t) (value));
4343

44-
static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t len) /* {{{ */
44+
static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t extra_len) /* {{{ */
4545
{
4646
union {
4747
phar_zip_extra_field_header header;
4848
phar_zip_unix3 unix3;
4949
phar_zip_unix_time time;
5050
} h;
51+
size_t len = extra_len;
5152
size_t read;
5253

53-
do {
54+
while (len) {
55+
size_t header_size;
56+
57+
if (len < sizeof(h.header)) {
58+
return FAILURE;
59+
}
5460
if (sizeof(h.header) != php_stream_read(fp, (char *) &h.header, sizeof(h.header))) {
5561
return FAILURE;
5662
}
63+
len -= sizeof(h.header);
64+
header_size = PHAR_GET_16(h.header.size);
65+
if (header_size > len) {
66+
return FAILURE;
67+
}
5768

5869
if (h.header.tag[0] == 'U' && h.header.tag[1] == 'T') {
5970
/* Unix timestamp header found.
@@ -62,7 +73,6 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16
6273
* We only store the modification time in the entry, so only read that.
6374
*/
6475
const size_t min_size = 5;
65-
uint16_t header_size = PHAR_GET_16(h.header.size);
6676
if (header_size >= min_size) {
6777
read = php_stream_read(fp, &h.time.flags, min_size);
6878
if (read != min_size) {
@@ -73,36 +83,48 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16
7383
entry->timestamp = PHAR_GET_32(h.time.time);
7484
}
7585

76-
len -= header_size + 4;
77-
7886
/* Consume remaining bytes */
79-
if (header_size != read) {
80-
php_stream_seek(fp, header_size - read, SEEK_CUR);
87+
if (header_size != read && -1 == php_stream_seek(fp, header_size - read, SEEK_CUR)) {
88+
return FAILURE;
8189
}
90+
len -= header_size;
8291
continue;
8392
}
8493
/* Fallthrough to next if to skip header */
8594
}
8695

8796
if (h.header.tag[0] != 'n' || h.header.tag[1] != 'u') {
8897
/* skip to next header */
89-
php_stream_seek(fp, PHAR_GET_16(h.header.size), SEEK_CUR);
90-
len -= PHAR_GET_16(h.header.size) + 4;
98+
if (header_size && -1 == php_stream_seek(fp, header_size, SEEK_CUR)) {
99+
return FAILURE;
100+
}
101+
len -= header_size;
91102
continue;
92103
}
93104

94105
/* unix3 header found */
95-
read = php_stream_read(fp, (char *) &(h.unix3.crc32), sizeof(h.unix3) - sizeof(h.header));
96-
len -= read + 4;
106+
size_t unix3_size = sizeof(h.unix3) - sizeof(h.header);
107+
size_t field_size = header_size;
108+
if (field_size == unix3_size - sizeof(h.unix3.crc32)) {
109+
/* Some archives omit the CRC32 from the unix3 size field. */
110+
field_size = unix3_size;
111+
}
112+
if (field_size < unix3_size || field_size > len) {
113+
return FAILURE;
114+
}
97115

98-
if (sizeof(h.unix3) - sizeof(h.header) != read) {
116+
read = php_stream_read(fp, (char *) &(h.unix3.crc32), unix3_size);
117+
if (unix3_size != read) {
99118
return FAILURE;
100119
}
101120

102-
if (PHAR_GET_16(h.unix3.size) > sizeof(h.unix3) - 4) {
121+
if (field_size > unix3_size) {
103122
/* skip symlink filename - we may add this support in later */
104-
php_stream_seek(fp, PHAR_GET_16(h.unix3.size) - sizeof(h.unix3.size), SEEK_CUR);
123+
if (-1 == php_stream_seek(fp, field_size - unix3_size, SEEK_CUR)) {
124+
return FAILURE;
125+
}
105126
}
127+
len -= field_size;
106128

107129
/* set permissions */
108130
entry->flags &= PHAR_ENT_COMPRESSION_MASK;
@@ -113,7 +135,7 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16
113135
entry->flags |= PHAR_GET_16(h.unix3.perms) & PHAR_ENT_PERM_MASK;
114136
}
115137

116-
} while (len);
138+
}
117139

118140
return SUCCESS;
119141
}

0 commit comments

Comments
 (0)