From 72dc583d62e73fc2e437cf2b20194c0c84d63c75 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 21 Apr 2020 18:13:44 +0200 Subject: [PATCH 1/3] Fix #79503: Memory leak on duplicate metadata --- ext/phar/tar.c | 2 ++ ext/phar/tests/bug79503.phar | Bin 0 -> 4001 bytes ext/phar/tests/bug79503.phpt | 16 ++++++++++++++++ 3 files changed, 18 insertions(+) create mode 100644 ext/phar/tests/bug79503.phar create mode 100644 ext/phar/tests/bug79503.phpt diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 7004676e0baf8..00eb647fe897d 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -181,10 +181,12 @@ static int phar_tar_process_metadata(phar_entry_info *entry, php_stream *fp) /* } if (entry->filename_len == sizeof(".phar/.metadata.bin")-1 && !memcmp(entry->filename, ".phar/.metadata.bin", sizeof(".phar/.metadata.bin")-1)) { + zval_ptr_dtor(&entry->phar->metadata); entry->phar->metadata = entry->metadata; ZVAL_UNDEF(&entry->metadata); } else if (entry->filename_len >= sizeof(".phar/.metadata/") + sizeof("/.metadata.bin") - 1 && NULL != (mentry = zend_hash_str_find_ptr(&(entry->phar->manifest), entry->filename + sizeof(".phar/.metadata/") - 1, entry->filename_len - (sizeof("/.metadata.bin") - 1 + sizeof(".phar/.metadata/") - 1)))) { /* transfer this metadata to the entry it refers */ + zval_ptr_dtor(&mentry->metadata); mentry->metadata = entry->metadata; ZVAL_UNDEF(&entry->metadata); } diff --git a/ext/phar/tests/bug79503.phar b/ext/phar/tests/bug79503.phar new file mode 100644 index 0000000000000000000000000000000000000000..d378c6f3dfff1eb7b8ba8d9fdbbd1a326aa6ecaa GIT binary patch literal 4001 zcmdNZ$Ve>GFD@xf(ksX)V4w*w00J{JGYqpq;$UEA%wT9_3RGiaXk=*402DJfGBs2% zz|cVhfzslV#3G=TG%$K77-?e81_~4LGCq;E@h4^?6N1u>* zXMf)SPaoGH4NYqWdplmPq2R4iHxd~Fz+4Z^|9ZKpC5b7CC5d`TnR!I&BwiUbmt$yv ziW?XhnSk;<3;@}NKr!Rd{Erc0)E6kWGBmPM@{Nr5bq#TJaSU;cSF*-w%V_?`88kyx zVl@Ac?E0Ui{7*z8|B42P&@#YEe*cGn0d@wpt^YxufhDQMCA8}aK->hX=<)Uk3^Dox fX2!twfRQnIej-WN2=5P&6jl@{W}-mvh|&rGAa`$= literal 0 HcmV?d00001 diff --git a/ext/phar/tests/bug79503.phpt b/ext/phar/tests/bug79503.phpt new file mode 100644 index 0000000000000..d4f480ec0ea88 --- /dev/null +++ b/ext/phar/tests/bug79503.phpt @@ -0,0 +1,16 @@ +--TEST-- +Bug #79503 (Memory leak on duplicate metadata) +--SKIPIF-- + +--FILE-- +getMessage(); +} +?> +--EXPECTF-- +phar error: "%s%ebug79503.phar" is a corrupted tar file (checksum mismatch of file "") From 335af72230c1a24b0085eba375e8f08324225d1d Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 22 Apr 2020 10:21:11 +0200 Subject: [PATCH 2/3] Bail out on duplicate metadata Duplicate metadata can only happen if someone tampers with the phar, so we can and should treat that as error. --- ext/phar/tar.c | 8 ++++++-- ext/phar/tests/bug79503.phpt | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 00eb647fe897d..dac055c2647a7 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -181,12 +181,16 @@ static int phar_tar_process_metadata(phar_entry_info *entry, php_stream *fp) /* } if (entry->filename_len == sizeof(".phar/.metadata.bin")-1 && !memcmp(entry->filename, ".phar/.metadata.bin", sizeof(".phar/.metadata.bin")-1)) { - zval_ptr_dtor(&entry->phar->metadata); + if (Z_TYPE(entry->phar->metadata) != IS_UNDEF) { + return FAILURE; + } entry->phar->metadata = entry->metadata; ZVAL_UNDEF(&entry->metadata); } else if (entry->filename_len >= sizeof(".phar/.metadata/") + sizeof("/.metadata.bin") - 1 && NULL != (mentry = zend_hash_str_find_ptr(&(entry->phar->manifest), entry->filename + sizeof(".phar/.metadata/") - 1, entry->filename_len - (sizeof("/.metadata.bin") - 1 + sizeof(".phar/.metadata/") - 1)))) { + if (Z_TYPE(mentry->metadata) != IS_UNDEF) { + return FAILURE; + } /* transfer this metadata to the entry it refers */ - zval_ptr_dtor(&mentry->metadata); mentry->metadata = entry->metadata; ZVAL_UNDEF(&entry->metadata); } diff --git a/ext/phar/tests/bug79503.phpt b/ext/phar/tests/bug79503.phpt index d4f480ec0ea88..874330fac743c 100644 --- a/ext/phar/tests/bug79503.phpt +++ b/ext/phar/tests/bug79503.phpt @@ -13,4 +13,4 @@ try { } ?> --EXPECTF-- -phar error: "%s%ebug79503.phar" is a corrupted tar file (checksum mismatch of file "") +phar error: tar-based phar "%s%ebug79503.phar" has invalid metadata in magic file ".phar/.metadata.bin" From 2073c0307909606a060a4d5b50d3284f69d9b88b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 22 Apr 2020 14:11:13 +0200 Subject: [PATCH 3/3] Fixed newly introduced mem leak --- ext/phar/tar.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/phar/tar.c b/ext/phar/tar.c index dac055c2647a7..5df5bfec7334f 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -182,12 +182,14 @@ static int phar_tar_process_metadata(phar_entry_info *entry, php_stream *fp) /* if (entry->filename_len == sizeof(".phar/.metadata.bin")-1 && !memcmp(entry->filename, ".phar/.metadata.bin", sizeof(".phar/.metadata.bin")-1)) { if (Z_TYPE(entry->phar->metadata) != IS_UNDEF) { + efree(metadata); return FAILURE; } entry->phar->metadata = entry->metadata; ZVAL_UNDEF(&entry->metadata); } else if (entry->filename_len >= sizeof(".phar/.metadata/") + sizeof("/.metadata.bin") - 1 && NULL != (mentry = zend_hash_str_find_ptr(&(entry->phar->manifest), entry->filename + sizeof(".phar/.metadata/") - 1, entry->filename_len - (sizeof("/.metadata.bin") - 1 + sizeof(".phar/.metadata/") - 1)))) { if (Z_TYPE(mentry->metadata) != IS_UNDEF) { + efree(metadata); return FAILURE; } /* transfer this metadata to the entry it refers */