[PHP-CVS] [php-src] master: ext/zip: fix extractTo()/getFrom*() success on CRC-corrupted entries

Author: David Carlier (devnexen)
Date: 2026-08-13T22:06:26+01:00

Commit: https://github.com/php/php-src/commit/2a2b337392a1e8d522026b3441360692db373192
Raw diff: https://github.com/php/php-src/commit/2a2b337392a1e8d522026b3441360692db373192.diff

ext/zip: fix extractTo()/getFrom*() success on CRC-corrupted entries

zip_fclose() returns positive error codes, so the extraction's n < 0 check
treated a failed read as success and wrote corrupt data to disk.
getFromName()/getFromIndex() read exactly sb.size bytes, never reaching end
of file where libzip validates the CRC, and returned the corrupt data
silently. Read errors now fail with a warning.

Close GH-23240

Changed paths:
  A  ext/zip/tests/oo_extract_crc.phpt
  A  ext/zip/tests/oo_get_from_crc_empty.phpt
  A  ext/zip/tests/oo_get_from_length.phpt
  M  NEWS
  M  ext/zip/php_zip.c


Diff:

diff --git a/NEWS b/NEWS
index 8ddc64181e45..db0775ae7f46 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or
     next() call on the inner generator). (iliaal)
 
+- Zip:
+  . Fixed ZipArchive::extractTo() and ZipArchive::getFrom*() reporting success
+    on corrupted entries. (David Carlier)
+
 27 Aug 2026, PHP 8.4.25
 
 - Core:
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 712468509cbc..3d6abde1c312 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -135,6 +135,30 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */
 # define CWD_STATE_ALLOC(l) emalloc(l)
 # define CWD_STATE_FREE(s)  efree(s)
 
+/* {{{ php_zip_file_error
+ Entry error code, plus its message when message is not NULL.
+ zip_error_t and its accessors only exist since libzip 1.0. */
+static int php_zip_file_error(struct zip_file *zf, const char **message)
+{
+#if LIBZIP_VERSION_MAJOR < 1
+	int zep, syp;
+
+	zip_file_error_get(zf, &zep, &syp);
+	if (message) {
+		*message = zip_file_strerror(zf);
+	}
+	return zep;
+#else
+	zip_error_t *err = zip_file_get_error(zf);
+
+	if (message) {
+		*message = zip_error_strerror(err);
+	}
+	return zip_error_code_zip(err);
+#endif
+}
+/* }}} */
+
 /* {{{ php_zip_extract_file */
 static int php_zip_extract_file(struct zip * za, char *dest, const char *file, size_t file_len, zip_int64_t idx)
 {
@@ -268,7 +292,21 @@ static int php_zip_extract_file(struct zip * za, char *dest, const char *file, s
 	n = 0;
 
 	while ((n=zip_fread(zf, b, sizeof(b))) > 0) {
-		php_stream_write(stream, b, n);
+		if (php_stream_write(stream, b, n) != n) {
+			n = -1;
+			break;
+		}
+	}
+
+	if (n < 0) {
+		const char *message;
+
+		if (php_zip_file_error(zf, &message) != ZIP_ER_OK) {
+			php_error_docref(NULL, E_WARNING, "Cannot extract \"%s\": \"%s\"", file, message);
+		}
+		php_stream_close(stream);
+		zip_fclose(zf);
+		goto done;
 	}
 
 	if (stream->wrapper->wops->stream_metadata) {
@@ -279,7 +317,7 @@ static int php_zip_extract_file(struct zip * za, char *dest, const char *file, s
 	}
 
 	php_stream_close(stream);
-	n = zip_fclose(zf);
+	n = zip_fclose(zf) == 0 ? 0 : -1;
 
 done:
 	efree(fullpath);
@@ -2953,10 +2991,6 @@ static void php_zip_get_from(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 		PHP_ZIP_STAT_INDEX(intern, index, flags, sb);
 	}
 
-	if (sb.size < 1) {
-		RETURN_EMPTY_STRING();
-	}
-
 	if (len < 1) {
 		len = sb.size;
 	}
@@ -2971,8 +3005,40 @@ static void php_zip_get_from(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
 	}
 
 	buffer = zend_string_safe_alloc(1, len, 0, 0);
-	zip_int64_t n = zip_fread(zf, ZSTR_VAL(buffer), ZSTR_LEN(buffer));
-	if (n < 1) {
+
+	/* zip_fread() may return short reads, a truncated entry must not pass for a complete one. */
+	zip_int64_t n = 0;
+	while ((zip_uint64_t)n < ZSTR_LEN(buffer)) {
+		zip_int64_t rd = zip_fread(zf, ZSTR_VAL(buffer) + n, ZSTR_LEN(buffer) - n);
+
+		if (rd < 0) {
+			n = -1;
+			break;
+		}
+		if (rd == 0) {
+			break;
+		}
+		n += rd;
+	}
+
+	if (n >= 0 && (zip_uint64_t)n == sb.size) {
+		/* The whole entry has been consumed, read past its last byte so that
+		 * libzip reaches the end of the stream and validates the CRC. */
+		char tmp;
+		if (zip_fread(zf, &tmp, 1) < 0) {
+			n = -1;
+		}
+	}
+	if (n < 0) {
+		const char *message;
+
+		php_zip_file_error(zf, &message);
+		php_error_docref(NULL, E_WARNING, "Cannot read entry: %s", message);
+		zip_fclose(zf);
+		zend_string_efree(buffer);
+		RETURN_FALSE;
+	}
+	if (n == 0) {
 		zip_fclose(zf);
 		zend_string_efree(buffer);
 		RETURN_EMPTY_STRING();
diff --git a/ext/zip/tests/oo_extract_crc.phpt b/ext/zip/tests/oo_extract_crc.phpt
new file mode 100644
index 000000000000..bcb31185c139
--- /dev/null
+++ b/ext/zip/tests/oo_extract_crc.phpt
@@ -0,0 +1,44 @@
+--TEST--
+ZipArchive::extractTo() and getFrom*() fail on a CRC-corrupted entry
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$dirname = __DIR__ . '/oo_extract_crc_dir';
+mkdir($dirname);
+$file = $dirname . '/corrupt.zip';
+$payload = str_repeat('A', 64) . 'PAYLOAD-END';
+
+$zip = new ZipArchive();
+$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+$zip->addFromString('a.txt', $payload);
+$zip->setCompressionName('a.txt', ZipArchive::CM_STORE);
+$zip->close();
+
+$raw = file_get_contents($file);
+$raw[strpos($raw, 'AAAA') + 2] = 'Z';
+file_put_contents($file, $raw);
+
+$zip = new ZipArchive();
+$zip->open($file);
+var_dump($zip->extractTo($dirname, 'a.txt'));
+var_dump($zip->getFromName('a.txt'));
+var_dump($zip->getFromIndex(0));
+$zip->close();
+?>
+--CLEAN--
+<?php
+$dirname = __DIR__ . '/oo_extract_crc_dir';
+@unlink($dirname . '/a.txt');
+@unlink($dirname . '/corrupt.zip');
+@rmdir($dirname);
+?>
+--EXPECTF--
+Warning: ZipArchive::extractTo(): Cannot extract "a.txt": "CRC error" in %s on line %d
+bool(false)
+
+Warning: ZipArchive::getFromName(): Cannot read entry: CRC error in %s on line %d
+bool(false)
+
+Warning: ZipArchive::getFromIndex(): Cannot read entry: CRC error in %s on line %d
+bool(false)
diff --git a/ext/zip/tests/oo_get_from_crc_empty.phpt b/ext/zip/tests/oo_get_from_crc_empty.phpt
new file mode 100644
index 000000000000..5e48059701e8
--- /dev/null
+++ b/ext/zip/tests/oo_get_from_crc_empty.phpt
@@ -0,0 +1,52 @@
+--TEST--
+ZipArchive::getFrom*() rejects a CRC-corrupted empty entry
+--EXTENSIONS--
+zip
+--SKIPIF--
+<?php
+/* libzip < 1.10.0 shortcuts empty entries and never checks their CRC. */
+if (version_compare(ZipArchive::LIBZIP_VERSION, '1.10.0', '<')) die('skip libzip < 1.10.0');
+?>
+--FILE--
+<?php
+$dirname = __DIR__ . '/oo_get_from_crc_empty_dir';
+mkdir($dirname);
+$file = $dirname . '/corrupt.zip';
+
+$zip = new ZipArchive();
+$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+$zip->addFromString('empty.txt', '');
+$zip->setCompressionName('empty.txt', ZipArchive::CM_STORE);
+$zip->close();
+
+/* Corrupt the CRC in both the local and central directory headers. */
+$raw = file_get_contents($file);
+for ($i = 0, $length = strlen($raw); $i + 3 < $length; $i++) {
+    $signature = substr($raw, $i, 4);
+    if ($signature === "PK\x03\x04") {
+        $raw[$i + 14] = "\x01";
+    } elseif ($signature === "PK\x01\x02") {
+        $raw[$i + 16] = "\x01";
+    }
+}
+file_put_contents($file, $raw);
+
+$zip = new ZipArchive();
+$zip->open($file);
+var_dump($zip->getFromName('empty.txt'));
+var_dump($zip->getFromIndex(0));
+$zip->close();
+?>
+--CLEAN--
+<?php
+$dirname = __DIR__ . '/oo_get_from_crc_empty_dir';
+@unlink($dirname . '/empty.txt');
+@unlink($dirname . '/corrupt.zip');
+@rmdir($dirname);
+?>
+--EXPECTF--
+Warning: ZipArchive::getFromName(): Cannot read entry: CRC error in %s on line %d
+bool(false)
+
+Warning: ZipArchive::getFromIndex(): Cannot read entry: CRC error in %s on line %d
+bool(false)
diff --git a/ext/zip/tests/oo_get_from_length.phpt b/ext/zip/tests/oo_get_from_length.phpt
new file mode 100644
index 000000000000..9eae83f3fec2
--- /dev/null
+++ b/ext/zip/tests/oo_get_from_length.phpt
@@ -0,0 +1,46 @@
+--TEST--
+ZipArchive::getFrom*() rejects an entry with an inconsistent uncompressed size
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$dirname = __DIR__ . '/oo_get_from_length_dir';
+mkdir($dirname);
+$file = $dirname . '/inconsistent.zip';
+
+$zip = new ZipArchive();
+$zip->open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+$zip->addFromString('a.txt', str_repeat('A', 10));
+$zip->setCompressionName('a.txt', ZipArchive::CM_STORE);
+$zip->close();
+
+/* Advertise 20 bytes in the central directory, but keep only 10 bytes. */
+$raw = file_get_contents($file);
+for ($i = 0, $length = strlen($raw); $i + 27 < $length; $i++) {
+    if (substr($raw, $i, 4) === "PK\x01\x02") {
+        $size = unpack('V', substr($raw, $i + 24, 4))[1];
+        $raw = substr_replace($raw, pack('V', $size + 10), $i + 24, 4);
+        break;
+    }
+}
+file_put_contents($file, $raw);
+
+$zip = new ZipArchive();
+$zip->open($file);
+var_dump($zip->getFromName('a.txt'));
+var_dump($zip->getFromIndex(0));
+$zip->close();
+?>
+--CLEAN--
+<?php
+$dirname = __DIR__ . '/oo_get_from_length_dir';
+@unlink($dirname . '/a.txt');
+@unlink($dirname . '/inconsistent.zip');
+@rmdir($dirname);
+?>
+--EXPECTF--
+Warning: ZipArchive::getFromName(): Cannot read entry: Zip archive inconsistent%s
+bool(false)
+
+Warning: ZipArchive::getFromIndex(): Cannot read entry: Zip archive inconsistent%s
+bool(false)