From e7c8e6cde021afd637ea535b0641a1851e57fb2a Mon Sep 17 00:00:00 2001
From: Stanislav Malyshev <stas@php.net>
Date: Mon, 12 Nov 2018 14:02:26 -0800
Subject: [PATCH] Fix bug #77143 - add more checks to buffer reads

---
 NEWS                         |   4 ++++
 ext/phar/phar.c              |  30 +++++++++++++++++++++---------
 ext/phar/tests/bug73768.phpt |   2 +-
 ext/phar/tests/bug77143.phar | Bin 0 -> 50 bytes
 ext/phar/tests/bug77143.phpt |  18 ++++++++++++++++++
 5 files changed, 44 insertions(+), 10 deletions(-)
 create mode 100644 ext/phar/tests/bug77143.phar
 create mode 100644 ext/phar/tests/bug77143.phpt

diff -Nur php-7.2.10/NEWS php-7.2.10_bak/NEWS
--- php-7.2.10/NEWS	2018-09-11 15:06:00.000000000 +0800
+++ php-7.2.10_bak/NEWS	2019-04-04 17:41:54.869000000 +0800
@@ -136,6 +136,10 @@
   . Fixed bug #76477 (Opcache causes empty return value).
     (Nikita, Laruence)
 
+- Phar:
+  . Fixed bug #77143 (Heap Buffer Overflow (READ: 4) in phar_parse_pharfile).
+    (Stas)
+
 - PGSQL:
   . Fixed bug #76548 (pg_fetch_result did not fetch the next row). (Anatol)

diff -Nur php-7.2.10/ext/phar/phar.c php-7.2.10_bak/ext/phar/phar.c
--- php-7.2.10/ext/phar/phar.c	2019-04-04 17:39:04.158000000 +0800
+++ php-7.2.10_bak/ext/phar/phar.c	2019-04-04 17:49:51.807000000 +0800
@@ -643,6 +643,18 @@
 /* }}}*/
 
 /**
+ * Size of fixed fields in the manifest.
+ * See: http://php.net/manual/en/phar.fileformat.phar.php
+ */
+#define MANIFEST_FIXED_LEN 18
+
+#define SAFE_PHAR_GET_32(buffer, endbuffer, var) \
+   if (UNEXPECTED(buffer + 4 > endbuffer)) { \
+	   MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest header)"); \
+   } \
+   PHAR_GET_32(buffer, var);
+
+/**
  * Does not check for a previously opened phar in the cache.
  *
  * Parse a new one and add it to the cache, returning either SUCCESS or
@@ -725,7 +737,7 @@
 	savebuf = buffer;
 	endbuffer = buffer + manifest_len;
 
-	if (manifest_len < 10 || manifest_len != php_stream_read(fp, buffer, manifest_len)) {
+    if (manifest_len < MANIFEST_FIXED_LEN || manifest_len != php_stream_read(fp, buffer, manifest_len)) {
 		MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest header)")
 	}
 
@@ -750,7 +762,7 @@
 		return FAILURE;
 	}
 
-	PHAR_GET_32(buffer, manifest_flags);
+	SAFE_PHAR_GET_32(buffer, endbuffer, manifest_flags);
 
 	manifest_flags &= ~PHAR_HDR_COMPRESSION_MASK;
 	manifest_flags &= ~PHAR_FILE_COMPRESSION_MASK;
@@ -970,13 +982,13 @@
 	}
 
 	/* extract alias */
-	PHAR_GET_32(buffer, tmp_len);
+	SAFE_PHAR_GET_32(buffer, endbuffer, tmp_len);
 
 	if (buffer + tmp_len > endbuffer) {
 		MAPPHAR_FAIL("internal corruption of phar \"%s\" (buffer overrun)");
 	}
 
-	if (manifest_len < 10 + tmp_len) {
+	if (manifest_len < MANIFEST_FIXED_LEN + tmp_len) {
 		MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest header)")
 	}
 
@@ -1014,7 +1026,7 @@
 	}
 
 	/* we have 5 32-bit items plus 1 byte at least */
-	if (manifest_count > ((manifest_len - 10 - tmp_len) / (5 * 4 + 1))) {
+	if (manifest_count > ((manifest_len - MANIFEST_FIXED_LEN - tmp_len) / (5 * 4 + 1))) {
 		/* prevent serious memory issues */
 		MAPPHAR_FAIL("internal corruption of phar \"%s\" (too many manifest entries for size of manifest)")
 	}
@@ -1023,12 +1035,12 @@
 	mydata->is_persistent = PHAR_G(persist);
 
 	/* check whether we have meta data, zero check works regardless of byte order */
-	PHAR_GET_32(buffer, len);
+	SAFE_PHAR_GET_32(buffer, endbuffer, len);
 	if (mydata->is_persistent) {
 		mydata->metadata_len = len;
-		if(!len) {
+		if (!len) {
 			/* FIXME: not sure why this is needed but removing it breaks tests */
-			PHAR_GET_32(buffer, len);
+			SAFE_PHAR_GET_32(buffer, endbuffer, len);
 		}
 	}
 	if(len > (size_t)(endbuffer - buffer)) {
diff -Nur php-7.2.10/ext/phar/tests/bug73768.phpt php-7.2.10_bak/ext/phar/tests/bug73768.phpt
--- php-7.2.10/ext/phar/tests/bug73768.phpt	2018-09-11 15:06:03.000000000 +0800
+++ php-7.2.10_bak/ext/phar/tests/bug73768.phpt	2019-04-04 17:50:51.796000000 +0800
@@ -13,4 +13,4 @@
 }
 ?>
 --EXPECTF--
-cannot load phar "%sbug73768.phar" with implicit alias "" under different alias "alias.phar"
+internal corruption of phar "%sbug73768.phar" (truncated manifest header)
diff --git a/ext/phar/tests/bug77143.phpt b/ext/phar/tests/bug77143.phpt
new file mode 100644
index 0000000..f9f80fc
--- /dev/null
+++ b/ext/phar/tests/bug77143.phpt
@@ -0,0 +1,18 @@
+--TEST--
+PHP bug #77143: Heap Buffer Overflow (READ: 4) in phar_parse_pharfile
+--INI--
+phar.readonly=0
+--SKIPIF--
+<?php if (!extension_loaded("phar")) die("skip"); ?>
+--FILE--
+<?php
+chdir(__DIR__);
+try {
+var_dump(new Phar('bug77143.phar',0,'project.phar'));
+echo "OK\n";
+} catch(UnexpectedValueException $e) {
+	echo $e->getMessage();
+}
+?>
+--EXPECTF--
+internal corruption of phar "%sbug77143.phar" (truncated manifest header)
-- 
2.1.4