php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #81420 ZipArchive::extractTo may extract outside of destination dir
Submitted: 2021-09-06 14:20 UTC Modified: 2021-09-21 04:36 UTC
From: cmb@php.net Assigned: stas (profile)
Status: Closed Package: Zip Related
PHP Version: 7.3 OS: Windows
Private report: No CVE-ID: 2021-21706
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: cmb@php.net
New email:
PHP Version: OS:

 

 [2021-09-06 14:20 UTC] cmb@php.net
Description:
------------
This has been reported as comment on a very old commit[1].

TL;DR: it is possible to construct ZIP archives containing files
which are placed outside the destination directory given to
ZipArchive::extractTo() because the implementation of
php_zip_make_relative_path() doesn't properly cater to absolute
directories on Windows; a path starting with a slash is not an
absolute path on Windows, but rather a relative path pointing to
the current volume.

I'm not sure whether this qualifies as security issue, but a very
similar issue regarding Phar::extractTo() (bug #70019) has been
handled as such.



Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-09-06 14:20 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2021-09-06 15:47 UTC] cmb@php.net
-PHP Version: 7.4Git-2021-09-06 (Git) +PHP Version: 7.3 -Assigned To: cmb +Assigned To: stas
 [2021-09-07 02:19 UTC] pajoye@php.net
The fix should work however I think something changed in virtual_file_ex. 
 
https://github.com/pierrejoye/php_zip/blob/1125a1e2b1d5f32862d241624afd6468fad0d6c5/php8/php_zip.c?plain=1#L149

I do remember having tested this exact case the last time we had this (Stas did a sec fix). virtual_file_ex, if I am not mistaken, should return an absolute path with drive in the path, not only /... . Or?
 [2021-09-08 15:42 UTC] cmb@php.net
> virtual_file_ex, if I am not mistaken, should return an absolute
> path with drive in the path, not only /... .

Not sure; there are some inconsistencies regarding such
"shortcuts", I think.  Anyway, the behavior is the same for PHP
7.4 (NTS and ZTS) and PHP 5.3.29.  And realpath("/") gives C:\ on
all these versions.
 [2021-09-14 16:44 UTC] cmb@php.net
> This has been reported as comment on a very old commit[1].

Forgot the link:
<https://github.com/php/php-src/commit/9976b5cd7f36d90b49d1dcf58ec6497f0e592b7d#commitcomment-55762818>
 [2021-09-16 10:33 UTC] derick@php.net
I'm reluctant to have this change in PHP 7.4, as it looks like a subtle change that can break (fragile) code. Especially because there also seems to be some confusion about what virtual_file_ex is supposed to do here.
 [2021-09-16 11:00 UTC] cmb@php.net
This is the least intrusive fix I can think of; changing the
behavior of virtual_ex() would affect a lot of functionality; this
change only affects ZipArchive::extractTo(), and there is no
difference on non Windows systems, since IS_ABSOLUTE_PATH(path,
path_len) is expanded to (IS_SLASH(path[0])) and
COPY_WHEN_ABSOLUTE(path) is expanded to 0 there.  Only for Windows
there is a behavioral change, namely that Zip paths with a leading
slash can no longer escape the given extraction directory.
 [2021-09-16 23:24 UTC] derick@php.net
Looks OK to me then.
 [2021-09-21 04:32 UTC] stas@php.net
-CVE-ID: +CVE-ID: 2021-21706
 [2021-09-21 04:36 UTC] git@php.net
Automatic comment on behalf of cmb69 (author) and smalyshev (committer)
Revision: https://github.com/php/php-src/commit/df2ceac25a43d72a0c25d3b415ae9eecc1ea195c
Log: Fix #81420: ZipArchive::extractTo extracts outside of destination
 [2021-09-21 04:36 UTC] git@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Nov 09 07:01:27 2024 UTC