Doc #77054 [Ver->Nab]: Directory traversal via ZipArchive::getNameIndex
| From: | stas@php.net | Date: | Fri, 26 Oct 2018 20:11:43 +0000 |
| Subject: | Doc #77054 [Ver->Nab]: Directory traversal via ZipArchive::getNameIndex | ||
| References: | 1 | Groups: | php.doc.bugs |
| Request: | Send a blank email to doc-bugs+get-16103@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=77054&edit=1
ID: 77054
Updated by: stas@php.net
Reported by: emil dot virkki at gmail dot com
Summary: Directory traversal via ZipArchive::getNameIndex
-Status: Verified
+Status: Not a bug
Type: Documentation Problem
Package: Zip Related
Operating System: macOS 10.13.6
PHP Version: 7.2.11
Block user comment: N
Private report: N
New Comment:
This looks like a problem in POC code, not in ZipArchive functions.
Previous Comments:
------------------------------------------------------------------------
[2018-10-26 12:27:47] cmb@php.net
Ah, now I understand! And yes, this should be documented.
------------------------------------------------------------------------
[2018-10-26 10:59:14] emil dot virkki at gmail dot com
Yes, that works correctly. The problem is with the getNameIndex() method, which returns the name as
"../index.php". When the PoC opens the file at $dst . '/' . $name, it opens the
wrong file.
------------------------------------------------------------------------
[2018-10-26 10:16:35] cmb@php.net
I cannot reproduce this; neither with PHP 7.2.11 NTS nor 7.2.11
ZTS. index.php is extracted into subdir/.
------------------------------------------------------------------------
[2018-10-24 18:15:41] emil dot virkki at gmail dot com
Description:
------------
ZipArchive::extractTo prevents extracting files outside the destination directory. However,
ZipArchive::getNameIndex will return the name without any such protections. This allows an attacker
to craft a Zip archive that can allow reading, writing or deleting arbitrary files depending on how
the return value of ZipArchive::getNameIndex is used.
If this is intentional behavior, the security aspects are not indicated in the documentation.
Test script:
---------------
<?php
$zip = new ZipArchive();
$zip->open("test.zip", ZipArchive::CREATE);
$zip->addFromString("subdir2/legit.txt", "Legit file\n");
$zip->addFromString("../index.php", "Attacker file\n");
$zip->close();
mkdir('subdir');
$zip = new ZipArchive();
$zip->open("test.zip");
for($i = 0; $i < $zip->numFiles; $i++) {
$name = $zip->getNameIndex($i);
echo "Extract $name\n";
$dst = __DIR__ . '/subdir';
if($zip->extractTo($dst, $name)) {
echo "The file contains:\n";
echo file_get_contents($dst . '/' . $name);
}
}
$zip->close();
Expected result:
----------------
Extract subdir2/legit.txt
The file contains:
Legit file
Extract index.php
The file contains:
Attacker file
Actual result:
--------------
Extract subdir2/legit.txt
The file contains:
Legit file
Extract ../index.php
The file contains:
<?php
$zip = new ZipArchive();
$zip->open("test.zip", ZipArchive::CREATE);
$zip->addFromString("subdir2/legit.txt", "Legit file\n");
$zip->addFromString("../index.php", "Attacker file\n");
$zip->close();
mkdir('subdir');
$zip = new ZipArchive();
$zip->open("test.zip");
for($i = 0; $i < $zip->numFiles; $i++) {
$name = $zip->getNameIndex($i);
echo "Extract $name\n";
$dst = __DIR__ . '/subdir';
if($zip->extractTo($dst, $name)) {
echo "The file contains:\n";
echo file_get_contents($dst . '/' . $name);
}
}
$zip->close();
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=77054&edit=1