Bug #73809 [Opn]: Phar Zip parse crash - mmap fail

From: Date: Tue, 01 Dec 2020 13:25:37 +0000
Subject: Bug #73809 [Opn]: Phar Zip parse crash - mmap fail
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-230763@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=73809&edit=1

 ID:                 73809
 Updated by:         cmb@php.net
 Reported by:        eyal dot itkin at gmail dot com
 Summary:            Phar Zip parse crash - mmap fail
 Status:             Open
 Type:               Bug
 Package:            PHAR related
 PHP Version:        7.1.0
-Assigned To:        
+Assigned To:        cmb
 Block user comment: N
 Private report:     N

 New Comment:

My assessment above wasn't correct, since this issue is only
about the .phar/signature.bin file, and restricting its size
appears to be reasonable.

However, this doesn't change the fact that this is not a security
issue, since PHP does not crash the *process*, but rather
terminates the *request* with a fatal error.


Previous Comments:
------------------------------------------------------------------------
[2020-11-26 15:22:40] cmb@php.net

Firstly, this is not related to a corrupt ZIP archive; any archive
which contains a file with very large uncompressed files can
trigger the reported behavior, assuming the script is run on a
32bit architecture, and memory_limit is set to a value too high
for such systems (in the given case memory_limit > 2GB).

This does not look like a security issue.  And we should not put
an arbitrary restriction on the uncompressed size of files in
archives (actually, we may want to support Zip64 some day).

Anyhow, passing unsanitized input to emalloc() is fine in this
case.  What is probaly not reasonable, is to attempt to fully
mmap() such large files at once.  Not long ago we landed a related
fix[1] regarding copying of streams where mmapping the whole file
yielded suboptimal performance.  It may make sense to try to move
this deeper into the streams layer (might not be possible without
BC break), or at least to let very large mmapping attempts fail.

[1] <https://github.com/php/php-src/commit/19c844594e40d79cea016b54f9ab3a367440b4c9>

------------------------------------------------------------------------
[2017-01-16 08:53:46] eyal dot itkin at gmail dot com

The limitations I mentioned before are hardcodes code checks in thr PHAR module. I don't know
what was the original design, but I  did wrote all of the code limitations from the module, from the
beginning of the handling to it's end.

In adsition, on my computer (standard config) it produces the attached crash (32 bit linux) and so I
reported it as a "crash" security report.

------------------------------------------------------------------------
[2017-01-16 08:48:37] stas@php.net

I was not asking who set the limitations, I was asking how you arrived at the conclusion these
limitations exist. 

> As @leigh mentioned, passing controlled sizes to emalloc() is a security risk

Repeating this statement does not make it more true. I do not see how it is a security risk, if you
think it is please explain it, not repeat it. I have no idea what "previous tickets" you
refer to, but in any case we're discussing this ticket, not any other ones.

------------------------------------------------------------------------
[2017-01-16 08:44:06] eyal dot itkin at gmail dot com

I don't know who set the current phar limitations. The fact is that there are hardcoded
limitations today, and the ZIP module lacks checks that could be done according to these
limitations, as I wrote in details in my prev comments.

As @leigh mentioned, passing controlled sizes to emalloc() is a security risk, that on my 32 bit
linux machine caused an mmap failure with the supplied .phar file. I really can't see the
difference between this ticket and previous DoS tickets that were caused from unsanitized calls to
emalloc() and were handled without such a questioning process.

------------------------------------------------------------------------
[2017-01-16 08:05:53] stas@php.net

> 1) TAR archive in the PHAR module can't be bigger than 512 bytes.
> 2) PHAR archive is limited to 100MB.
> 3) The ZIP archive in the PHAR module has an overall limit of 64KB.

I'm not sure I understand. Where these limitations come from? I know PHAR manifest can't
be larger than 100MB but I'm not sure why the whole phar can't be? 

I also don't see why passing any value to emalloc is a problem - we have memory limits for a
reason. I'm not sure why on your system it produces mmap error - I don't have any data on
the OS/build here in the ticket. In fact, when I run the current code on the same file this is what
I get:

Fatal error: Uncaught UnexpectedValueException: internal corruption of phar
"/Users/smalyshev/Downloads/example_hostile.phar" (truncated manifest entry)

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=73809


--
Edit this bug report at https://bugs.php.net/bug.php?id=73809&edit=1


Thread (1 message)

  • cmb@php.net
  • Unknown Message
    • cmb@php.net
« previous php.bugs (#230763) next »