php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #77054 Directory traversal via ZipArchive::getNameIndex
Submitted: 2018-10-24 18:15 UTC Modified: 2018-10-26 20:11 UTC
From: emil dot virkki at gmail dot com Assigned:
Status: Not a bug Package: Zip Related
PHP Version: 7.2.11 OS: macOS 10.13.6
Private report: No CVE-ID: None
 [2018-10-24 18:15 UTC] 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();


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-10-26 10:16 UTC] 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-26 10:59 UTC] 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 12:27 UTC] cmb@php.net
-Status: Open +Status: Verified -Type: Security +Type: Documentation Problem
 [2018-10-26 12:27 UTC] cmb@php.net
Ah, now I understand!  And yes, this should be documented.
 [2018-10-26 20:11 UTC] stas@php.net
-Status: Verified +Status: Not a bug
 [2018-10-26 20:11 UTC] stas@php.net
This looks like a problem in POC code, not in ZipArchive functions.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Thu Aug 22 09:01:27 2019 UTC