Skip to content

Commit b292254

Browse files
devnexendevnexen
committed
Merge branch 'PHP-8.4' into PHP-8.5
* PHP-8.4: ext/zip: fix extractTo()/getFrom*() success on CRC-corrupted entries
2 parents daeeed8 + 2a2b337 commit b292254

4 files changed

Lines changed: 216 additions & 8 deletions

File tree

‎ext/zip/php_zip.c‎

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,30 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */
129129
# define CWD_STATE_ALLOC(l) emalloc(l)
130130
# define CWD_STATE_FREE(s) efree(s)
131131

132+
/* {{{ php_zip_file_error
133+
Entry error code, plus its message when message is not NULL.
134+
zip_error_t and its accessors only exist since libzip 1.0. */
135+
static int php_zip_file_error(struct zip_file *zf, const char **message)
136+
{
137+
#if LIBZIP_VERSION_MAJOR < 1
138+
int zep, syp;
139+
140+
zip_file_error_get(zf, &zep, &syp);
141+
if (message) {
142+
*message = zip_file_strerror(zf);
143+
}
144+
return zep;
145+
#else
146+
zip_error_t *err = zip_file_get_error(zf);
147+
148+
if (message) {
149+
*message = zip_error_strerror(err);
150+
}
151+
return zip_error_code_zip(err);
152+
#endif
153+
}
154+
/* }}} */
155+
132156
/* {{{ php_zip_extract_file */
133157
static int php_zip_extract_file(struct zip * za, char *dest, const char *file, size_t file_len, zip_int64_t idx)
134158
{
@@ -262,7 +286,21 @@ static int php_zip_extract_file(struct zip * za, char *dest, const char *file, s
262286
n = 0;
263287

264288
while ((n=zip_fread(zf, b, sizeof(b))) > 0) {
265-
php_stream_write(stream, b, n);
289+
if (php_stream_write(stream, b, n) != n) {
290+
n = -1;
291+
break;
292+
}
293+
}
294+
295+
if (n < 0) {
296+
const char *message;
297+
298+
if (php_zip_file_error(zf, &message) != ZIP_ER_OK) {
299+
php_error_docref(NULL, E_WARNING, "Cannot extract \"%s\": \"%s\"", file, message);
300+
}
301+
php_stream_close(stream);
302+
zip_fclose(zf);
303+
goto done;
266304
}
267305

268306
if (stream->wrapper->wops->stream_metadata) {
@@ -273,7 +311,7 @@ static int php_zip_extract_file(struct zip * za, char *dest, const char *file, s
273311
}
274312

275313
php_stream_close(stream);
276-
n = zip_fclose(zf);
314+
n = zip_fclose(zf) == 0 ? 0 : -1;
277315

278316
done:
279317
efree(fullpath);
@@ -2905,10 +2943,6 @@ static void php_zip_get_from(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
29052943
PHP_ZIP_STAT_INDEX(intern, index, flags, sb);
29062944
}
29072945

2908-
if (sb.size < 1) {
2909-
RETURN_EMPTY_STRING();
2910-
}
2911-
29122946
if (len < 1) {
29132947
len = sb.size;
29142948
}
@@ -2923,8 +2957,40 @@ static void php_zip_get_from(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
29232957
}
29242958

29252959
buffer = zend_string_safe_alloc(1, len, 0, 0);
2926-
zip_int64_t n = zip_fread(zf, ZSTR_VAL(buffer), ZSTR_LEN(buffer));
2927-
if (n < 1) {
2960+
2961+
/* zip_fread() may return short reads, a truncated entry must not pass for a complete one. */
2962+
zip_int64_t n = 0;
2963+
while ((zip_uint64_t)n < ZSTR_LEN(buffer)) {
2964+
zip_int64_t rd = zip_fread(zf, ZSTR_VAL(buffer) + n, ZSTR_LEN(buffer) - n);
2965+
2966+
if (rd < 0) {
2967+
n = -1;
2968+
break;
2969+
}
2970+
if (rd == 0) {
2971+
break;
2972+
}
2973+
n += rd;
2974+
}
2975+
2976+
if (n >= 0 && (zip_uint64_t)n == sb.size) {
2977+
/* The whole entry has been consumed, read past its last byte so that
2978+
* libzip reaches the end of the stream and validates the CRC. */
2979+
char tmp;
2980+
if (zip_fread(zf, &tmp, 1) < 0) {
2981+
n = -1;
2982+
}
2983+
}
2984+
if (n < 0) {
2985+
const char *message;
2986+
2987+
php_zip_file_error(zf, &message);
2988+
php_error_docref(NULL, E_WARNING, "Cannot read entry: %s", message);
2989+
zip_fclose(zf);
2990+
zend_string_efree(buffer);
2991+
RETURN_FALSE;
2992+
}
2993+
if (n == 0) {
29282994
zip_fclose(zf);
29292995
zend_string_efree(buffer);
29302996
RETURN_EMPTY_STRING();

‎ext/zip/tests/oo_extract_crc.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
ZipArchive::extractTo() and getFrom*() fail on a CRC-corrupted entry
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
$dirname = __DIR__ . '/oo_extract_crc_dir';
8+
mkdir($dirname);
9+
$file = $dirname . '/corrupt.zip';
10+
$payload = str_repeat('A', 64) . 'PAYLOAD-END';
11+
12+
$zip = new ZipArchive();
13+
$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
14+
$zip->addFromString('a.txt', $payload);
15+
$zip->setCompressionName('a.txt', ZipArchive::CM_STORE);
16+
$zip->close();
17+
18+
$raw = file_get_contents($file);
19+
$raw[strpos($raw, 'AAAA') + 2] = 'Z';
20+
file_put_contents($file, $raw);
21+
22+
$zip = new ZipArchive();
23+
$zip->open($file);
24+
var_dump($zip->extractTo($dirname, 'a.txt'));
25+
var_dump($zip->getFromName('a.txt'));
26+
var_dump($zip->getFromIndex(0));
27+
$zip->close();
28+
?>
29+
--CLEAN--
30+
<?php
31+
$dirname = __DIR__ . '/oo_extract_crc_dir';
32+
@unlink($dirname . '/a.txt');
33+
@unlink($dirname . '/corrupt.zip');
34+
@rmdir($dirname);
35+
?>
36+
--EXPECTF--
37+
Warning: ZipArchive::extractTo(): Cannot extract "a.txt": "CRC error" in %s on line %d
38+
bool(false)
39+
40+
Warning: ZipArchive::getFromName(): Cannot read entry: CRC error in %s on line %d
41+
bool(false)
42+
43+
Warning: ZipArchive::getFromIndex(): Cannot read entry: CRC error in %s on line %d
44+
bool(false)

‎ext/zip/tests/oo_get_from_crc_empty.phpt‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
ZipArchive::getFrom*() rejects a CRC-corrupted empty entry
3+
--EXTENSIONS--
4+
zip
5+
--SKIPIF--
6+
<?php
7+
/* libzip < 1.10.0 shortcuts empty entries and never checks their CRC. */
8+
if (version_compare(ZipArchive::LIBZIP_VERSION, '1.10.0', '<')) die('skip libzip < 1.10.0');
9+
?>
10+
--FILE--
11+
<?php
12+
$dirname = __DIR__ . '/oo_get_from_crc_empty_dir';
13+
mkdir($dirname);
14+
$file = $dirname . '/corrupt.zip';
15+
16+
$zip = new ZipArchive();
17+
$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
18+
$zip->addFromString('empty.txt', '');
19+
$zip->setCompressionName('empty.txt', ZipArchive::CM_STORE);
20+
$zip->close();
21+
22+
/* Corrupt the CRC in both the local and central directory headers. */
23+
$raw = file_get_contents($file);
24+
for ($i = 0, $length = strlen($raw); $i + 3 < $length; $i++) {
25+
$signature = substr($raw, $i, 4);
26+
if ($signature === "PK\x03\x04") {
27+
$raw[$i + 14] = "\x01";
28+
} elseif ($signature === "PK\x01\x02") {
29+
$raw[$i + 16] = "\x01";
30+
}
31+
}
32+
file_put_contents($file, $raw);
33+
34+
$zip = new ZipArchive();
35+
$zip->open($file);
36+
var_dump($zip->getFromName('empty.txt'));
37+
var_dump($zip->getFromIndex(0));
38+
$zip->close();
39+
?>
40+
--CLEAN--
41+
<?php
42+
$dirname = __DIR__ . '/oo_get_from_crc_empty_dir';
43+
@unlink($dirname . '/empty.txt');
44+
@unlink($dirname . '/corrupt.zip');
45+
@rmdir($dirname);
46+
?>
47+
--EXPECTF--
48+
Warning: ZipArchive::getFromName(): Cannot read entry: CRC error in %s on line %d
49+
bool(false)
50+
51+
Warning: ZipArchive::getFromIndex(): Cannot read entry: CRC error in %s on line %d
52+
bool(false)

‎ext/zip/tests/oo_get_from_length.phpt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
ZipArchive::getFrom*() rejects an entry with an inconsistent uncompressed size
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
$dirname = __DIR__ . '/oo_get_from_length_dir';
8+
mkdir($dirname);
9+
$file = $dirname . '/inconsistent.zip';
10+
11+
$zip = new ZipArchive();
12+
$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
13+
$zip->addFromString('a.txt', str_repeat('A', 10));
14+
$zip->setCompressionName('a.txt', ZipArchive::CM_STORE);
15+
$zip->close();
16+
17+
/* Advertise 20 bytes in the central directory, but keep only 10 bytes. */
18+
$raw = file_get_contents($file);
19+
for ($i = 0, $length = strlen($raw); $i + 27 < $length; $i++) {
20+
if (substr($raw, $i, 4) === "PK\x01\x02") {
21+
$size = unpack('V', substr($raw, $i + 24, 4))[1];
22+
$raw = substr_replace($raw, pack('V', $size + 10), $i + 24, 4);
23+
break;
24+
}
25+
}
26+
file_put_contents($file, $raw);
27+
28+
$zip = new ZipArchive();
29+
$zip->open($file);
30+
var_dump($zip->getFromName('a.txt'));
31+
var_dump($zip->getFromIndex(0));
32+
$zip->close();
33+
?>
34+
--CLEAN--
35+
<?php
36+
$dirname = __DIR__ . '/oo_get_from_length_dir';
37+
@unlink($dirname . '/a.txt');
38+
@unlink($dirname . '/inconsistent.zip');
39+
@rmdir($dirname);
40+
?>
41+
--EXPECTF--
42+
Warning: ZipArchive::getFromName(): Cannot read entry: Zip archive inconsistent%s
43+
bool(false)
44+
45+
Warning: ZipArchive::getFromIndex(): Cannot read entry: Zip archive inconsistent%s
46+
bool(false)

0 commit comments

Comments
 (0)