Skip to content

Commit d1ffc25

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

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
@@ -164,6 +164,7 @@ PHP NEWS
164164
ignored. (ndossche)
165165
. Fixed a bypass of the magic ".phar" directory protection in
166166
Phar::addEmptyDir() for paths starting with "/.phar". (Weilin Du)
167+
. Fixed an integer underflow when parsing ZIP extra fields. (Weilin Du)
167168
. Phar::addEmptyDir() now allows non-magic directory names that merely
168169
share the ".phar" prefix. (Weilin Du)
169170
. Support overridden methods in SplFileInfo for getMTime() and getPathname()

‎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
@@ -39,19 +39,30 @@ static inline void phar_write_16(char buffer[2], uint32_t value)
3939
# define PHAR_SET_32(var, value) phar_write_32(var, (uint32_t) (value));
4040
# define PHAR_SET_16(var, value) phar_write_16(var, (uint16_t) (value));
4141

42-
static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t len) /* {{{ */
42+
static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t extra_len) /* {{{ */
4343
{
4444
union {
4545
phar_zip_extra_field_header header;
4646
phar_zip_unix3 unix3;
4747
phar_zip_unix_time time;
4848
} h;
49+
size_t len = extra_len;
4950
size_t read;
5051

51-
do {
52+
while (len) {
53+
size_t header_size;
54+
55+
if (len < sizeof(h.header)) {
56+
return FAILURE;
57+
}
5258
if (sizeof(h.header) != php_stream_read(fp, (char *) &h.header, sizeof(h.header))) {
5359
return FAILURE;
5460
}
61+
len -= sizeof(h.header);
62+
header_size = PHAR_GET_16(h.header.size);
63+
if (header_size > len) {
64+
return FAILURE;
65+
}
5566

5667
if (h.header.tag[0] == 'U' && h.header.tag[1] == 'T') {
5768
/* Unix timestamp header found.
@@ -60,7 +71,6 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry
6071
* We only store the modification time in the entry, so only read that.
6172
*/
6273
const size_t min_size = 5;
63-
uint16_t header_size = PHAR_GET_16(h.header.size);
6474
if (header_size >= min_size) {
6575
read = php_stream_read(fp, &h.time.flags, min_size);
6676
if (read != min_size) {
@@ -71,36 +81,48 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry
7181
entry->timestamp = PHAR_GET_32(h.time.time);
7282
}
7383

74-
len -= header_size + 4;
75-
7684
/* Consume remaining bytes */
77-
if (header_size != read) {
78-
php_stream_seek(fp, header_size - read, SEEK_CUR);
85+
if (header_size != read && -1 == php_stream_seek(fp, header_size - read, SEEK_CUR)) {
86+
return FAILURE;
7987
}
88+
len -= header_size;
8089
continue;
8190
}
8291
/* Fallthrough to next if to skip header */
8392
}
8493

8594
if (h.header.tag[0] != 'n' || h.header.tag[1] != 'u') {
8695
/* skip to next header */
87-
php_stream_seek(fp, PHAR_GET_16(h.header.size), SEEK_CUR);
88-
len -= PHAR_GET_16(h.header.size) + 4;
96+
if (header_size && -1 == php_stream_seek(fp, header_size, SEEK_CUR)) {
97+
return FAILURE;
98+
}
99+
len -= header_size;
89100
continue;
90101
}
91102

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

96-
if (sizeof(h.unix3) - sizeof(h.header) != read) {
114+
read = php_stream_read(fp, (char *) &(h.unix3.crc32), unix3_size);
115+
if (unix3_size != read) {
97116
return FAILURE;
98117
}
99118

100-
if (PHAR_GET_16(h.unix3.size) > sizeof(h.unix3) - 4) {
119+
if (field_size > unix3_size) {
101120
/* skip symlink filename - we may add this support in later */
102-
php_stream_seek(fp, PHAR_GET_16(h.unix3.size) - sizeof(h.unix3.size), SEEK_CUR);
121+
if (-1 == php_stream_seek(fp, field_size - unix3_size, SEEK_CUR)) {
122+
return FAILURE;
123+
}
103124
}
125+
len -= field_size;
104126

105127
/* set permissions */
106128
entry->flags &= PHAR_ENT_COMPRESSION_MASK;
@@ -111,7 +133,7 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry
111133
entry->flags |= PHAR_GET_16(h.unix3.perms) & PHAR_ENT_PERM_MASK;
112134
}
113135

114-
} while (len);
136+
}
115137

116138
return SUCCESS;
117139
}

0 commit comments

Comments
 (0)