php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

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
View Developer Edit
 [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-06 15:47 UTC] cmb@php.net 
Suggested fix:
<https://gist.github.com/cmb69/d393788723f8dcca10219f7f45ab009d>.
 [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: Thu Nov 21 10:01:29 2024 UTC