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 #81211 Symlinks are followed when creating PHAR archive
Submitted: 2021-06-30 12:59 UTC Modified: 2021-08-26 20:59 UTC
From: trungpa at viettel dot com dot vn Assigned: stas (profile)
Status: Closed Package: PHAR related
PHP Version: 8.1Git-2021-06-30 (Git) OS: MacOS, Linux
Private report: No CVE-ID: None
View Developer Edit
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: trungpa at viettel dot com dot vn
New email:
PHP Version: OS:

 

 [2021-06-30 12:59 UTC] trungpa at viettel dot com dot vn 
Description:
------------
When constructing PHAR/TAR archive using PharData::buildFromDirectory and PharData::buildFromIterator, file paths returned by RecursiveIteratorIterator is validated to ensure that files are from inside the specified directory: https://github.com/php/php-src/blob/aff365871aec54c9a556d7667f131b8638d20194/ext/phar/phar_object.c#L1504
However, due to the check being too lenient, it can be bypassed with the following values:
- base:  /usr/foo
- fname: /usr/foobar
or
- base:  /tmp
- fname: /var/tmp

Test script:
---------------
<?php
mkdir('/tmp/usr');
mkdir('/tmp/usr/foobar');
mkdir('/tmp/usr/foo');

file_put_contents('/tmp/usr/foobar/file', 'this file should NOT be included in the archive!');
symlink('/tmp/usr/foobar/file', '/tmp/usr/foo/symlink');

$archive = new PharData('/tmp/usr/archive.tar');
$archive->buildFromDirectory('/tmp/usr/foo');
// $archive->buildFromIterator(new RecursiveDirectoryIterator('/tmp/usr/foo', FilesystemIterator::SKIP_DOTS), '/tmp/usr/foo');

var_dump(file_get_contents("phar:///tmp/usr/archive.tar/bar/file"));


Expected result:
----------------
UnexpectedValueException is thrown.

Fatal error: Uncaught UnexpectedValueException: Iterator RecursiveIteratorIterator returned a path "/tmp/usr/foobar/file" that is not in the base directory "/tmp/usr/foo"...

Actual result:
--------------
/tmp/usr/foobar/file (which is outside of the base directory /tmp/usr/foo) is included in archive.tar

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-07-01 12:59 UTC] cmb@php.net
-Assigned To: +Assigned To: stas
 [2021-07-01 12:59 UTC] cmb@php.net 
Thanks for reporting this issue!

Suggested patch for PHP-7.3 or PHP-7.4:
<https://gist.github.com/cmb69/46e4238e01a565603b4e25d1964547e9>.
 [2021-07-02 04:06 UTC] trungpa at viettel dot com dot vn 
The check will pass with fname=/foo/bar/fooxbar and base=/fooxbar.
I think we also need to ensure that fname starts with base.
Something like fname == strstr(fname, base).
https://gist.github.com/trungPa/71b557a2a775508b16119e9c21aade7d
 [2021-07-02 11:33 UTC] cmb@php.net 
> The check will pass with fname=/foo/bar/fooxbar and base=/fooxbar.

Oh, right!  I wonder why strstr() is used here at all.  A strncmp()
appears more suitable.  Even if fname is a relative path, base is
always expanded, so strstr() would fail anyway.

Then again, I think that fname needs to be normalized; otherwise
there are issues with slashes as directory separators on Windows,
which are usually supported.  But this is not really related to
the issue at hand.
 [2021-07-02 11:59 UTC] trungpa at viettel dot com dot vn 
Agree! strncmp() looks like a better alternative to strstr().
 [2021-08-23 06:29 UTC] stas@php.net 
@cmb do you want to update this patch with strncmp or leave it as is?
 [2021-08-23 11:44 UTC] cmb@php.net 
I've updated the patch to use strncmp() and to work for PHP 7.3:
<https://gist.github.com/cmb69/404cbe40a77e2e18d3331db5e9cc5092>.
 [2021-08-23 12:46 UTC] trungpa at viettel dot com dot vn 
@cmb would the test fail on linux because of the "\" on line #86-87? I remember having this problem and had to replace it with %s%ebug81211%efoobar%efile
 [2021-08-24 06:40 UTC] git@php.net 
Automatic comment on behalf of cmb69 (author) and smalyshev (committer)
Revision: https://github.com/php/php-src/commit/2ff853aa113e52637c85e28d1a03df1aa2d747b5
Log: Fix #81211: Symlinks are followed when creating PHAR archive
 [2021-08-24 06:40 UTC] git@php.net
-Status: Assigned +Status: Closed
 [2021-08-24 09:13 UTC] cmb@php.net 
> @cmb would the test fail on linux because of the "\" on line
> #86-87?

Right.  That has been fixed:
<https://github.com/php/php-src/commit/b815645aac76b494dc119fa6b88de32fa9bcccf1>.
 [2021-08-26 18:14 UTC] trungpa at viettel dot com dot vn 
Is there any plan to request a cve id for the issue?
 [2021-08-26 19:42 UTC] stas@php.net 
I'm not sure this needs a CVE - ultimately it is a local filesystem action which must be triggered by the user code. Granted, if the user is not careful it could lead to a security consequences (like adding files that should not be there to an archive) but this does not look like it needs a special notification.
 [2021-08-26 20:59 UTC] trungpa at viettel dot com dot vn 
This bug might cause problems in a local context. For example, an LPE, when a malicious local user has read/write permission to the input directory and the output archive, they can read files under the application context. But I agree that the possibility is low, the strstr(fname, base) also makes the bug limited and it's unlikely to be exploited remotely. I won't request an id from MITRE, so it's up to you to decide.
 [2021-09-20 19:39 UTC] 1234211qqq at gmail dot com 
a
 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved. 		Last updated: Mon Mar 31 07:01:29 2025 UTC