]> granicus.if.org Git - php/commitdiff
MFPECL: fix security vulnerability in phar's handling of long tar filenames
authorGreg Beaver <cellog@php.net>
Thu, 4 Jun 2009 20:00:01 +0000 (20:00 +0000)
committerGreg Beaver <cellog@php.net>
Thu, 4 Jun 2009 20:00:01 +0000 (20:00 +0000)
ext/phar/tar.c
ext/phar/tests/tar/bignames_overflow.phpt [new file with mode: 0644]
ext/phar/tests/tar/files/make.dangerous.tar.php.inc [new file with mode: 0644]

index 0ff00471bddf93381eec14599bb5c0b0c65c1f3a..85c06499006ca30607a0e047f5beb756f1d9d389 100644 (file)
@@ -330,16 +330,19 @@ bail:
 
                if (!old && hdr->prefix[0] != 0) {
                        char name[256];
+                       int i, j;
 
-                       strcpy(name, hdr->prefix);
-                       /* remove potential buffer overflow */
-                       if (hdr->name[99]) {
-                               strncat(name, hdr->name, 100);
-                       } else {
-                               strcat(name, hdr->name);
+                       for (i = 0; i < 155; i++) {
+                               name[i] = hdr->prefix[i];
+                               if (name[i] == '\0') {
+                                       break;
+                               }
+                       }
+                       for (j = 0; j < 100; j++) {
+                               name[i+j] = hdr->name[j];
                        }
 
-                       entry.filename_len = strlen(hdr->prefix) + 100;
+                       entry.filename_len = i+j;
 
                        if (name[entry.filename_len - 1] == '/') {
                                /* some tar programs store directories with trailing slash */
@@ -347,8 +350,16 @@ bail:
                        }
                        entry.filename = pestrndup(name, entry.filename_len, myphar->is_persistent);
                } else {
-                       entry.filename = pestrdup(hdr->name, myphar->is_persistent);
-                       entry.filename_len = strlen(entry.filename);
+                       int i;
+
+                       /* calculate strlen, which can be no longer than 100 */
+                       for (i = 0; i < 100; i++) {
+                               if (hdr->name[i] == '\0') {
+                                       break;
+                               }
+                       }
+                       entry.filename_len = i;
+                       entry.filename = pestrndup(hdr->name, i, myphar->is_persistent);
 
                        if (entry.filename[entry.filename_len - 1] == '/') {
                                /* some tar programs store directories with trailing slash */
diff --git a/ext/phar/tests/tar/bignames_overflow.phpt b/ext/phar/tests/tar/bignames_overflow.phpt
new file mode 100644 (file)
index 0000000..8087311
--- /dev/null
@@ -0,0 +1,40 @@
+--TEST--
+Phar: tar with huge filenames, buffer overflow
+--SKIPIF--
+<?php if (!extension_loaded("phar")) die("skip"); ?>
+--INI--
+phar.require_hash=0
+--FILE--
+<?php
+$fname = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.tar';
+$fname2 = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.2.tar';
+$pname = 'phar://' . $fname;
+
+include dirname(__FILE__) . '/files/make.dangerous.tar.php.inc';
+
+$tar = new danger_tarmaker($fname, 'none');
+$tar->init();
+$tar->addFile(str_repeat('a', 101), 'hi');
+$tar->addFile(str_repeat('a', 255), 'hi2');
+$tar->close();
+
+$p1 = new PharData($fname);
+foreach ($p1 as $file) {
+       echo $file->getFileName(), "\n";
+}
+echo $p1[str_repeat('a', 101)]->getContent() . "\n";
+echo $p1[str_repeat('a', 255)]->getContent() . "\n";
+
+?>
+===DONE===
+--CLEAN--
+<?php
+unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.tar');
+unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.2.tar');
+?>
+--EXPECT--
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+hi
+hi2
+===DONE===
diff --git a/ext/phar/tests/tar/files/make.dangerous.tar.php.inc b/ext/phar/tests/tar/files/make.dangerous.tar.php.inc
new file mode 100644 (file)
index 0000000..deeaa41
--- /dev/null
@@ -0,0 +1,170 @@
+<?php
+// stolen from PEAR2_Pyrus_Developer_Creator_Tar by Greg Beaver, the original author, for use in unit tests
+// this tarmaker makes a malicious tar with a header designed to overflow the buffer
+class danger_tarmaker
+{
+    /**
+     * Path to archive file
+     *
+     * @var string
+     */
+    protected $archive;
+    /**
+     * Temporary stream used for creating the archive
+     *
+     * @var stream
+     */
+    protected $tmp;
+    protected $path;
+    protected $compress;
+    function __construct($path, $compress = 'zlib')
+    {
+        $this->compress = $compress;
+        if ($compress === 'bz2' && !function_exists('bzopen')) {
+            throw new PEAR2_Pyrus_Developer_Creator_Exception(
+                'bzip2 extension not available');
+        }
+        if ($compress === 'zlib' && !function_exists('gzopen')) {
+            throw new PEAR2_Pyrus_Developer_Creator_Exception(
+                'zlib extension not available');
+        }
+        $this->path = $path;
+    }
+
+    /**
+     * save a file inside this package
+     * 
+     * This code is modified from Vincent Lascaux's File_Archive
+     * package, which is licensed under the LGPL license.
+     * @param string relative path within the package
+     * @param string|resource file contents or open file handle
+     */
+    function addFile($path, $fileOrStream, $stat = null)
+    {
+        clearstatcache();
+        if ($stat === null) {
+            if (is_resource($fileOrStream)) {
+                $stat = fstat($fileOrStream);
+            } else {
+                $stat = array(
+                    'mode' => 0x8000 + 0644,
+                    'uid' => 0,
+                    'gid' => 0,
+                    'size' => strlen($fileOrStream),
+                    'mtime' => time(),
+                );
+            }
+        }
+
+        $link = null;
+        if ($stat['mode'] & 0x4000) {
+            $type = 5;        // Directory
+        } else if ($stat['mode'] & 0x8000) {
+            $type = 0;        // Regular
+        } else if ($stat['mode'] & 0xA000) {
+            $type = 1;        // Link
+            $link = @readlink($current);
+        } else {
+            $type = 9;        // Unknown
+        }
+
+        $filePrefix = '';
+        if (strlen($path) > 255) {
+            throw new Exception(
+                "$path is too long, must be 255 characters or less"
+            );
+        } else if (strlen($path) > 100) {
+            $filePrefix = substr($path, 0, strlen($path)-100);
+            $path = substr($path, -100);
+        }
+
+        $block = pack('a100a8a8a8a12A12',
+                $path,
+                '12345678', // have a mode that allows the name to overflow
+                sprintf('%6s ',decoct($stat['uid'])),
+                sprintf('%6s ',decoct($stat['gid'])),
+                sprintf('%11s ',decoct($stat['size'])),
+                sprintf('%11s ',decoct($stat['mtime']))
+            );
+
+        $blockend = pack('a1a100a6a2a32a32a8a8a155a12',
+            $type,
+            $link,
+            'ustar',
+            '00',
+            'Pyrus',
+            'Pyrus',
+            '',
+            '',
+            $filePrefix,
+            '123456789abc'); // malicious block
+
+        $checkheader = array_merge(str_split($block), str_split($blockend));
+        if (!function_exists('_pear2tarchecksum')) {
+            function _pear2tarchecksum($a, $b) {return $a + ord($b);}
+        }
+        $checksum = 256; // 8 * ord(' ');
+        $checksum += array_reduce($checkheader, '_pear2tarchecksum');
+
+        $checksum = pack('a8', sprintf('%6s ', decoct($checksum)));
+
+        fwrite($this->tmp, (binary)$block . $checksum . $blockend, 512);
+        if (is_resource($fileOrStream)) {
+            stream_copy_to_stream($fileOrStream, $this->tmp);
+            if ($stat['size'] % 512) {
+                fwrite($this->tmp, (binary)str_repeat("\0", 512 - $stat['size'] % 512));
+            }
+        } else {
+            fwrite($this->tmp, (binary)$fileOrStream);
+            if (strlen($fileOrStream) % 512) {
+                fwrite($this->tmp, (binary)str_repeat("\0", 512 - strlen($fileOrStream) % 512));
+            }
+        }
+    }
+
+    /**
+     * Initialize the package creator
+     */
+    function init()
+    {
+        switch ($this->compress) {
+            case 'zlib' :
+                $this->tmp = gzopen($this->path, 'wb');
+                break;
+            case 'bz2' :
+                $this->tmp = bzopen($this->path, 'w');
+                break;
+            case 'none' :
+                $this->tmp = fopen($this->path, 'wb');
+                break;
+            default :
+                throw new Exception(
+                    'unknown compression type ' . $this->compress);
+        }
+    }
+
+    /**
+     * Create an internal directory, creating parent directories as needed
+     * 
+     * @param string $dir
+     */
+    function mkdir($dir)
+    {
+        $this->addFile($dir, "", array(
+                    'mode' => 0x4000 + 0644,
+                    'uid' => 0,
+                    'gid' => 0,
+                    'size' => 0,
+                    'mtime' => time(),
+                ));
+    }
+
+    /**
+     * Finish saving the package
+     */
+    function close()
+    {
+        fwrite($this->tmp, pack('a1024', ''));
+        fclose($this->tmp);
+    }
+}
\ No newline at end of file