rework errors slightly. Add buffer overflow check for manifest, so we don't create...

diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index 04a0475e68b0a2b2b6ca9c788ccd0f74fe45a5be..0ba5978d9553d245869b8cdee887b664e3c6df76 100644 (file)
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -163,17 +163,18 @@ PHP_METHOD(PHP_Archive, mapPhar)
                php_stream_close(fp);\
                php_error_docref(NULL TSRMLS_CC, E_ERROR, msg, fname);\
                return;
-#define MAPPHAR_FAIL(msg) efree(buffer);\
+#define MAPPHAR_FAIL(msg) efree(savebuf);\
                MAPPHAR_ALLOC_FAIL(msg)
 
        // check for ?>\n and increment accordingly
        if (-1 == php_stream_seek(fp, halt_offset, SEEK_SET)) {
-               MAPPHAR_FAIL("cannot seek to __HALT_COMPILER(); location in phar \"%s\"")
+               MAPPHAR_ALLOC_FAIL("cannot seek to __HALT_COMPILER(); location in phar \"%s\"")
        }
 
        if (FALSE == (buffer = (char *) emalloc(4))) {
                MAPPHAR_ALLOC_FAIL("memory allocation failed in phar \"%s\"")
        }
+       savebuf = buffer;
        if (3 != php_stream_read(fp, buffer, 3)) {
                MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest)")
        }
@@ -202,9 +203,9 @@ PHP_METHOD(PHP_Archive, mapPhar)
 
        i = 0;
 #define PHAR_GET_VAL(var)                      \
-       if (buffer > endbuffer) {\
+       if (buffer > endbuffer) {               \
                MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest)")\
-       }\
+       }                                       \
        unpack_var = (char *) &var;             \
        var = 0;                                \
        for (i = 0; i < 4; i++) {               \
@@ -217,8 +218,12 @@ PHP_METHOD(PHP_Archive, mapPhar)
        endbuffer = buffer;
        PHAR_GET_VAL(manifest_len)
        buffer -= 4;
+       if (manifest_len > 1048576) {
+               /* prevent serious memory issues by limiting manifest to at most 1 MB in length */
+               MAPPHAR_FAIL("manifest cannot be larger than 1 MB in phar \"%s\"")
+       }
        if (FALSE == (buffer = (char *) erealloc(buffer, manifest_len))) {
-               MAPPHAR_ALLOC_FAIL("memory allocation failed in phar \"%s\"")
+               MAPPHAR_FAIL("memory allocation failed in phar \"%s\"")
        }
        savebuf = buffer;
        // set the test pointer

diff --git a/ext/phar/tests/002.phpt b/ext/phar/tests/002.phpt
index 8220fae1bb9824785a6f88f5b5e2af964aec13b4..db5d42f28d2e19dba3b2344b69f5c6aeabdc344b 100644 (file)
--- a/ext/phar/tests/002.phpt
+++ b/ext/phar/tests/002.phpt
@@ -10,8 +10,7 @@ PHP_Archive::mapPhar(5, 5);
 PHP_Archive::mapPhar(5, 'hio');
 PHP_Archive::mapPhar(5, 'hio', 'hi');
 PHP_Archive::mapPhar(5, 'hio', true, 5, 5);
-__HALT_COMPILER();
-?>
+__HALT_COMPILER(); ?>
 --EXPECTF--
 Warning: PHP_Archive::mapPhar() expects at least 3 parameters, 0 given in %s on line %d
 

diff --git a/ext/phar/tests/005.phpt b/ext/phar/tests/005.phpt
index 2ec6c315a4aeb624243050bd61ad0e8c6e8be808..62f761f986c2762f57d0c48c030a328dd27a7269 100644 (file)
--- a/ext/phar/tests/005.phpt
+++ b/ext/phar/tests/005.phpt
@@ -5,7 +5,6 @@ PHP_Archive::mapPhar improper parameters
 --FILE--
 <?php
 PHP_Archive::mapPhar(5, 'hio', false);
-__HALT_COMPILER();
-?>
+__HALT_COMPILER(); ?>
 --EXPECTF--
 Fatal error: PHP_Archive::mapPhar(): internal corruption of phar "%s" (truncated manifest) in %s on line %d
\ No newline at end of file

diff --git a/ext/phar/tests/006.phpt b/ext/phar/tests/006.phpt
index 07992a4b70affd0d2dff0249106dbd76437aa8de..d66f2efedebe3f16af04fe40dea5f57c180bb52a 100644 (file)
--- a/ext/phar/tests/006.phpt
+++ b/ext/phar/tests/006.phpt
@@ -5,6 +5,6 @@ PHP_Archive::mapPhar improper parameters
 --FILE--
 <?php
 PHP_Archive::mapPhar(5, 'hio', false);
-__HALT_COMPILER(); ?>()a
+__HALT_COMPILER(); ?>()
 --EXPECTF--
 Fatal error: PHP_Archive::mapPhar(): internal corruption of phar "%s" (truncated manifest) in %s on line %d
\ No newline at end of file

diff --git a/ext/phar/tests/007.phpt b/ext/phar/tests/007.phpt
new file mode 100644 (file)
index 0000000..4772364
--- /dev/null
+++ b/ext/phar/tests/007.phpt
@@ -0,0 +1,10 @@
+--TEST--
+PHP_Archive::mapPhar improper parameters
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip";?>
+--FILE--
+<?php
+PHP_Archive::mapPhar(5, 'hio', false);
+__HALT_COMPILER(); ?>~~~~
+--EXPECTF--
+Fatal error: PHP_Archive::mapPhar(): manifest cannot be larger than 1 MB in phar "%s" in %s on line %d
\ No newline at end of file