Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:44453
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain chiaraquartet.net from 209.85.216.189 cause and error)
Message-ID: <4A41CBFD.7000102@chiaraquartet.net>
Date: Wed, 24 Jun 2009 01:47:25 -0500
User-Agent: Thunderbird 2.0.0.21 (X11/20090318)
MIME-Version: 1.0
To: internals@lists.php.net
Content-Type: multipart/mixed;
 boundary="------------000302050704060003000300"
Subject: [PATCH] fix for ext/phar openssl signature verification in tar archives
From: greg@chiaraquartet.net (Greg Beaver)

--------------000302050704060003000300
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

I just discovered two critical errors in tar's signature verification 
handling that affects archives signed with an openssl signature.  The 
attached patch fixes these issues, and adds a test for the openssl 
functionality (this slipped through the cracks somehow).

This is an important issue for anyone who wants to create true signed 
archives with ext/phar, and so I hope it will make it into 5.3.0.  If it 
is too late, I would at least want the patch to tar.c to be in the 
release notes if at all possible.  The patch attached is against 
PHP_5_3, and is a trivial one to merge to HEAD and pecl/phar.

Can I commit?

Thanks,
Greg

--------------000302050704060003000300
Content-Type: text/plain;
 name="fix.openssl.patch.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="fix.openssl.patch.txt"

Index: ext/phar/tar.c
===================================================================
RCS file: /repository/php-src/ext/phar/tar.c,v
retrieving revision 1.55.2.29
diff -u -r1.55.2.29 tar.c
--- ext/phar/tar.c	4 Jun 2009 19:59:09 -0000	1.55.2.29
+++ ext/phar/tar.c	24 Jun 2009 06:43:38 -0000
@@ -255,6 +255,8 @@
 			phar_tar_number(hdr->size, sizeof(hdr->size));
 
 		if (((!old && hdr->prefix[0] == 0) || old) && strlen(hdr->name) == sizeof(".phar/signature.bin")-1 && !strncmp(hdr->name, ".phar/signature.bin", sizeof(".phar/signature.bin")-1)) {
+			off_t curloc;
+
 			if (size > 511) {
 				if (error) {
 					spprintf(error, 4096, "phar error: tar-based phar \"%s\" has signature that is larger than 511 bytes, cannot process", fname);
@@ -264,6 +266,7 @@
 				phar_destroy_phar_data(myphar TSRMLS_CC);
 				return FAILURE;
 			}
+			curloc = php_stream_tell(fp);
 			read = php_stream_read(fp, buf, size);
 			if (read != size) {
 				if (error) {
@@ -280,7 +283,7 @@
 #else
 # define PHAR_GET_32(buffer) (php_uint32) *(buffer)
 #endif
-			if (FAILURE == phar_verify_signature(fp, php_stream_tell(fp) - size - 512, PHAR_GET_32(buf), buf + 8, PHAR_GET_32(buf + 4), fname, &myphar->signature, &myphar->sig_len, error TSRMLS_CC)) {
+			if (FAILURE == phar_verify_signature(fp, php_stream_tell(fp) - size - 512, PHAR_GET_32(buf), buf + 8, size - 8, fname, &myphar->signature, &myphar->sig_len, error TSRMLS_CC)) {
 				if (error) {
 					char *save = *error;
 					spprintf(error, 4096, "phar error: tar-based phar \"%s\" signature cannot be verified: %s", fname, save);
@@ -288,11 +291,11 @@
 				}
 				goto bail;
 			}
+			php_stream_seek(fp, curloc + 512, SEEK_SET);
 			/* signature checked out, let's ensure this is the last file in the phar */
-			size = ((size+511)&~511) + 512;
 			if (((hdr->typeflag == '\0') || (hdr->typeflag == TAR_FILE)) && size > 0) {
 				/* this is not good enough - seek succeeds even on truncated tars */
-				php_stream_seek(fp, size, SEEK_CUR);
+				php_stream_seek(fp, 512, SEEK_CUR);
 				if ((uint)php_stream_tell(fp) > totalsize) {
 					if (error) {
 						spprintf(error, 4096, "phar error: \"%s\" is a corrupted tar file (truncated)", fname);
Index: ext/phar/tests/tar/tar_openssl_hash.phpt
===================================================================
RCS file: ext/phar/tests/tar/tar_openssl_hash.phpt
diff -N ext/phar/tests/tar/tar_openssl_hash.phpt
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ext/phar/tests/tar/tar_openssl_hash.phpt	24 Jun 2009 06:43:40 -0000
@@ -0,0 +1,22 @@
+--TEST--
+Phar: tar archive, require_hash=1, OpenSSL hash
+--SKIPIF--
+<?php if (!extension_loaded('phar')) die('skip'); ?>
+<?php if (!extension_loaded("spl")) die("skip SPL not available"); ?>
+<?php if (!extension_loaded("zlib")) die("skip zlib not available"); ?>
+<?php if (!extension_loaded("openssl")) die("skip openssl not available"); ?>
+--INI--
+phar.readonly=1
+phar.require_hash=1
+--FILE--
+<?php
+try {
+	$phar = new PharData(dirname(__FILE__) . '/files/P1-1.0.0.tgz');
+} catch (Exception $e) {
+	echo $e->getMessage()."\n";
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
Index: ext/phar/tests/tar/files/P1-1.0.0.tgz
===================================================================
RCS file: ext/phar/tests/tar/files/P1-1.0.0.tgz
diff -N ext/phar/tests/tar/files/P1-1.0.0.tgz
Binary files /dev/null and P1-1.0.0.tgz differ
Index: ext/phar/tests/tar/files/P1-1.0.0.tgz.pubkey
===================================================================
RCS file: ext/phar/tests/tar/files/P1-1.0.0.tgz.pubkey
diff -N ext/phar/tests/tar/files/P1-1.0.0.tgz.pubkey
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ext/phar/tests/tar/files/P1-1.0.0.tgz.pubkey	24 Jun 2009 06:43:40 -0000
@@ -0,0 +1,9 @@
+-----BEGIN PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4drcwddPs6LmIbdT1ifT
+Ev8HXh1Fk1yNusCDoCX6mYkgqvCmx02F/9k5q7n6CPblTcF5mdDI8kcRrUHmyXtD
+9X0d7RN7BakZMPH5KPaNkXiXsI9YGSb39AnZgYw01n6u0W6Ohha+KwOsrxkKCF4u
+LjPLQAlM+3uD8y9Tz2fF+pAE901kHrd3ue7a5i5EtW0bzl5QfxnwFZXAO0StQ9dF
+slzibRH+1pFjMRxDnlgYmLQF6jMWm9Ty6x9UH9HZ3E3F9QZEQVXWT9y/pe30HcAX
+YxAGZjPIx19UNPF5C+Nps6MjxNRht0pGXTL9sptYoiNjRiXAS0y4FM+8K6xvBIOF
+ZQIDAQAB
+-----END PUBLIC KEY-----

--------------000302050704060003000300--