From 42e0e5dfb74f1f58962fb74f8f6f04493b4858d6 Mon Sep 17 00:00:00 2001 From: Greg Beaver Date: Fri, 24 Jul 2009 15:42:17 +0000 Subject: [PATCH] fix signature generation/validation for zip archives by phar extension, fix a few edge cases where memory was leaked on error conditions by missing efree() --- NEWS | 1 + ext/phar/phar_object.c | 6 - .../tests/zip/phar_setsignaturealgo2.phpt | 113 ++++++++++++ ext/phar/util.c | 11 ++ ext/phar/zip.c | 162 +++++++++++++++++- 5 files changed, 281 insertions(+), 12 deletions(-) create mode 100644 ext/phar/tests/zip/phar_setsignaturealgo2.phpt diff --git a/NEWS b/NEWS index 89a7bb35da..84e96651c4 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ PHP NEWS Functors. (Christian Seiler) - Fixed open_basedir circumvention for mail.log. (Maksymilian Arciemowicz, Stas) +- Fixed signature generation/validation for zip archives in ext/phar. (Greg) - Fixed bug #49032 (SplFileObject::fscanf() variables passed by reference). (Jani) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 285ebafc86..98a1379bb8 100755 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -3012,12 +3012,6 @@ PHP_METHOD(Phar, setSignatureAlgorithm) return; } - if (phar_obj->arc.archive->is_zip) { - zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, - "Cannot set signature algorithm, not possible with zip-based phar archives"); - return; - } - if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "l|s", &algo, &key, &key_len) != SUCCESS) { return; } diff --git a/ext/phar/tests/zip/phar_setsignaturealgo2.phpt b/ext/phar/tests/zip/phar_setsignaturealgo2.phpt new file mode 100644 index 0000000000..372f7ddc8e --- /dev/null +++ b/ext/phar/tests/zip/phar_setsignaturealgo2.phpt @@ -0,0 +1,113 @@ +--TEST-- +Phar::setSupportedSignatures() with hash, zip-based +--SKIPIF-- + + +--INI-- +phar.require_hash=0 +phar.readonly=0 +--FILE-- +getSignature()); +$p->setSignatureAlgorithm(Phar::MD5); + +copy($fname, $fname2); +$p = new Phar($fname2); +var_dump($p->getSignature()); + +$p->setSignatureAlgorithm(Phar::SHA1); + +copy($fname2, $fname3); +$p = new Phar($fname3); +var_dump($p->getSignature()); + +try { +$p->setSignatureAlgorithm(Phar::SHA256); +copy($fname3, $fname4); +$p = new Phar($fname4); +var_dump($p->getSignature()); +} catch (Exception $e) { +echo $e->getMessage(); +} +try { +$p->setSignatureAlgorithm(Phar::SHA512); +copy($fname4, $fname5); +$p = new Phar($fname5); +var_dump($p->getSignature()); +} catch (Exception $e) { +echo $e->getMessage(); +} +try { +$keys=openssl_pkey_new(); +openssl_pkey_export($keys, $privkey); +$pubkey=openssl_pkey_get_details($keys); +$p->setSignatureAlgorithm(Phar::OPENSSL, $privkey); + +copy($fname5, $fname6); +file_put_contents($fname6 . '.pubkey', $pubkey['key']); +$p = new Phar($fname6); +var_dump($p->getSignature()); +} catch (Exception $e) { +echo $e->getMessage(); +} +?> +===DONE=== +--CLEAN-- + +--EXPECTF-- +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(5) "SHA-1" +} +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(3) "MD5" +} +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(5) "SHA-1" +} +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(7) "SHA-256" +} +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(7) "SHA-512" +} +array(2) { + ["hash"]=> + string(%d) "%s" + ["hash_type"]=> + string(7) "OpenSSL" +} +===DONE=== diff --git a/ext/phar/util.c b/ext/phar/util.c index 2edfe773af..434a37e7f0 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -1665,6 +1665,11 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, off_t end, zval_dtor(zdata); zval_dtor(zsig); zval_dtor(zkey); + zval_dtor(openssl); + efree(openssl); + efree(zdata); + efree(zkey); + efree(zsig); return FAILURE; } @@ -1678,6 +1683,9 @@ static int phar_call_openssl_signverify(int is_sign, php_stream *fp, off_t end, zval_dtor(zkey); zval_dtor(openssl); efree(openssl); + efree(zdata); + efree(zkey); + efree(zsig); return FAILURE; } @@ -1797,6 +1805,9 @@ int phar_verify_signature(php_stream *fp, size_t end_of_phar, php_uint32 sig_typ efree(pfile); if (!pfp || !(pubkey_len = php_stream_copy_to_mem(pfp, &pubkey, PHP_STREAM_COPY_ALL, 0)) || !pubkey) { + if (pfp) { + php_stream_close(pfp); + } if (error) { spprintf(error, 0, "openssl public key could not be read"); } diff --git a/ext/phar/zip.c b/ext/phar/zip.c index ee365d0d12..181d561fc4 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -172,6 +172,7 @@ int phar_parse_zipfile(php_stream *fp, char *fname, int fname_len, char *alias, phar_archive_data *mydata = NULL; phar_entry_info entry = {0}; char *p = buf, *ext, *actual_alias = NULL; + char *metadata = NULL; size = php_stream_tell(fp); @@ -222,7 +223,6 @@ int phar_parse_zipfile(php_stream *fp, char *fname, int fname_len, char *alias, /* read in archive comment, if any */ if (PHAR_GET_16(locator.comment_len)) { - char *metadata; metadata = p + sizeof(locator); @@ -299,6 +299,27 @@ foundit: entry.is_zip = 1; entry.fp_type = PHAR_FP; entry.is_persistent = mydata->is_persistent; +#define PHAR_ZIP_FAIL_FREE(errmsg, save) \ + zend_hash_destroy(&mydata->manifest); \ + mydata->manifest.arBuckets = 0; \ + zend_hash_destroy(&mydata->mounted_dirs); \ + mydata->mounted_dirs.arBuckets = 0; \ + zend_hash_destroy(&mydata->virtual_dirs); \ + mydata->virtual_dirs.arBuckets = 0; \ + php_stream_close(fp); \ + if (mydata->metadata) { \ + zval_dtor(mydata->metadata); \ + } \ + if (error) { \ + spprintf(error, 4096, "phar error: %s in zip-based phar \"%s\"", errmsg, mydata->fname); \ + } \ + pefree(mydata->fname, mydata->is_persistent); \ + if (mydata->alias) { \ + pefree(mydata->alias, mydata->is_persistent); \ + } \ + pefree(mydata, mydata->is_persistent); \ + efree(save); \ + return FAILURE; #define PHAR_ZIP_FAIL(errmsg) \ zend_hash_destroy(&mydata->manifest); \ mydata->manifest.arBuckets = 0; \ @@ -374,6 +395,53 @@ foundit: entry.is_dir = 0; } + if (entry.filename_len == sizeof(".phar/signature.bin")-1 && !strncmp(entry.filename, ".phar/signature.bin", sizeof(".phar/signature.bin")-1)) { + size_t read; + php_stream *sigfile; + + pefree(entry.filename, entry.is_persistent); + sigfile = php_stream_fopen_tmpfile(); + + php_stream_seek(fp, 0, SEEK_SET); + /* copy file contents + local headers and zip comment, if any, to be hashed for signature */ + phar_stream_copy_to_stream(fp, sigfile, entry.header_offset, NULL); + if (metadata) { + php_stream_write(sigfile, metadata, PHAR_GET_16(locator.comment_len)); + } + php_stream_seek(fp, sizeof(phar_zip_file_header) + entry.header_offset + entry.filename_len + PHAR_GET_16(zipentry.extra_len), SEEK_SET); + read = php_stream_read(fp, buf, entry.uncompressed_filesize); + if (read != entry.uncompressed_filesize) { + php_stream_close(sigfile); + PHAR_ZIP_FAIL("signature cannot be read"); + } + mydata->sig_flags = PHAR_GET_32(buf); + if (FAILURE == phar_verify_signature(sigfile, php_stream_tell(sigfile), mydata->sig_flags, buf + 8, entry.uncompressed_filesize - 8, fname, &mydata->signature, &mydata->sig_len, error TSRMLS_CC)) { + if (error) { + char *save; + php_stream_close(sigfile); + spprintf(&save, 4096, "signature cannot be verified: %s", *error); + efree(*error); + PHAR_ZIP_FAIL_FREE(save, save); + } else { + php_stream_close(sigfile); + PHAR_ZIP_FAIL("signature cannot be verified"); + } + } + php_stream_close(sigfile); + /* signature checked out, let's ensure this is the last file in the phar */ + read = php_stream_read(fp, buf, 4); + + if (read != 4) { + PHAR_ZIP_FAIL("corrupted zip file (truncated)"); + } + + if (!memcmp(buf, "PK\5\6", 4)) { + PHAR_ZIP_FAIL("entries exist after signature, invalid phar"); + } + + continue; + } + phar_add_virtual_dirs(mydata, entry.filename, entry.filename_len TSRMLS_CC); if (PHAR_GET_16(zipentry.extra_len)) { @@ -1000,6 +1068,73 @@ continue_dir: } /* }}} */ +static int phar_zip_applysignature(phar_archive_data *phar, struct _phar_zip_pass *pass, + smart_str *metadata TSRMLS_DC) /* {{{ */ +{ + /* add signature for executable tars or tars explicitly set with setSignatureAlgorithm */ + if (!phar->is_data || phar->sig_flags) { + int signature_length; + char *signature, sigbuf[8]; + phar_entry_info entry = {0}; + php_stream *newfile; + off_t tell; + + newfile = php_stream_fopen_tmpfile(); + tell = php_stream_tell(pass->filefp); + /* copy the local files and the zip comment to generate the hash */ + php_stream_seek(pass->filefp, 0, SEEK_SET); + phar_stream_copy_to_stream(pass->filefp, newfile, tell, NULL); + if (metadata->c) { + php_stream_write(pass->filefp, metadata->c, metadata->len); + } + + if (FAILURE == phar_create_signature(phar, newfile, &signature, &signature_length, pass->error TSRMLS_CC)) { + if (pass->error) { + char *save = *(pass->error); + spprintf(pass->error, 0, "phar error: unable to write signature to zip-based phar: %s", save); + efree(save); + } + + php_stream_close(newfile); + return FAILURE; + } + + entry.filename = ".phar/signature.bin"; + entry.filename_len = sizeof(".phar/signature.bin")-1; + entry.fp = php_stream_fopen_tmpfile(); + entry.fp_type = PHAR_MOD; + entry.is_modified = 1; + + PHAR_SET_32(sigbuf, phar->sig_flags); + PHAR_SET_32(sigbuf + 4, signature_length); + + if (8 != (int)php_stream_write(entry.fp, sigbuf, 8) || signature_length != (int)php_stream_write(entry.fp, signature, signature_length)) { + efree(signature); + if (pass->error) { + spprintf(pass->error, 0, "phar error: unable to write signature to zip-based phar %s", phar->fname); + } + + php_stream_close(newfile); + return FAILURE; + } + + efree(signature); + entry.uncompressed_filesize = entry.compressed_filesize = signature_length + 8; + entry.phar = phar; + /* throw out return value and write the signature */ + phar_zip_changed_apply((void *)&entry, (void *)pass TSRMLS_CC); + php_stream_close(newfile); + + if (pass->error && *(pass->error)) { + /* error is set by writeheaders */ + php_stream_close(newfile); + return FAILURE; + } + } /* signature */ + return SUCCESS; +} +/* }}} */ + int phar_zip_flush(phar_archive_data *phar, char *user_stub, long len, int defaultstub, char **error TSRMLS_DC) /* {{{ */ { char *pos; @@ -1214,10 +1349,21 @@ fperror: memset(&eocd, 0, sizeof(eocd)); strncpy(eocd.signature, "PK\5\6", 4); - PHAR_SET_16(eocd.counthere, zend_hash_num_elements(&phar->manifest)); - PHAR_SET_16(eocd.count, zend_hash_num_elements(&phar->manifest)); + if (phar->sig_flags) { + PHAR_SET_16(eocd.counthere, zend_hash_num_elements(&phar->manifest) + 1); + PHAR_SET_16(eocd.count, zend_hash_num_elements(&phar->manifest) + 1); + } else { + PHAR_SET_16(eocd.counthere, zend_hash_num_elements(&phar->manifest)); + PHAR_SET_16(eocd.count, zend_hash_num_elements(&phar->manifest)); + } zend_hash_apply_with_argument(&phar->manifest, phar_zip_changed_apply, (void *) &pass TSRMLS_CC); + if (phar->metadata) { + /* set phar metadata */ + PHP_VAR_SERIALIZE_INIT(metadata_hash); + php_var_serialize(&main_metadata_str, &phar->metadata, &metadata_hash TSRMLS_CC); + PHP_VAR_SERIALIZE_DESTROY(metadata_hash); + } if (temperr) { if (error) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: %s", phar->fname, temperr); @@ -1226,6 +1372,9 @@ fperror: temperror: php_stream_close(pass.centralfp); nocentralerror: + if (phar->metadata) { + smart_str_free(&main_metadata_str); + } php_stream_close(pass.filefp); if (closeoldfile) { php_stream_close(oldfile); @@ -1233,6 +1382,10 @@ nocentralerror: return EOF; } + if (FAILURE == phar_zip_applysignature(phar, &pass, &main_metadata_str TSRMLS_CC)) { + goto temperror; + } + /* save zip */ cdir_size = php_stream_tell(pass.centralfp); cdir_offset = php_stream_tell(pass.filefp); @@ -1255,9 +1408,6 @@ nocentralerror: if (phar->metadata) { /* set phar metadata */ - PHP_VAR_SERIALIZE_INIT(metadata_hash); - php_var_serialize(&main_metadata_str, &phar->metadata, &metadata_hash TSRMLS_CC); - PHP_VAR_SERIALIZE_DESTROY(metadata_hash); PHAR_SET_16(eocd.comment_len, main_metadata_str.len); if (sizeof(eocd) != php_stream_write(pass.filefp, (char *)&eocd, sizeof(eocd))) { -- 2.40.0