The bug is relevant only to the zip option in the phar module.

Please provide example_hostile.phar

Added the example_hostile.phar at this link: http://www.cs.tau.ac.il/~eyalitki/Upload/73809/
Added also the python script that generated it.

I'm not sure I understand this one - what you mean by "extensive allocations"? Which number would be considered "extensive"? Of course it is possible that the file is too big for PHP to process - this is a legitimate error condition, not a security issue. Could you explain the security issue you see here?

There are several options to limit the ZIP's size in the PHAR module. In all of the cases we want to trade a FATAL exception (i.e. a crash) with a normal exception error that indicates the ZIP is too large.

Some background:
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.

The only thing that is not limited is the size of the signature.bin inside the ZIP archive.

In addition, this signature is supposed to hold a supported signature: 
1) MD5     - 16 bytes
2) SHA1    - 20 bytes
3) SHA256  - 32 bytes
4) SHA512  - 64 bytes
5) OPENSSL - not exactly clear to me how many bytes the SSL library is expecting

I think it is safe to limit the uncompressed signature size to 64KB:
1) Cryptographic signatures can not be compressed efficiently. Meaning that the size of a compressed valid signature will almost be as large as the original uncompressed signature
2) The overall size for the ZIP archive is 64KB
3) As for today OPENSSL does not support any signature of size > 10KB.

OK, if this is about changing fatal error to more friendly error handling, it's not a security issue.

I don't think I agree.
The fix is to return an error (via regular expression) instead of crashing the PHP process with trace of:
mmap() failed: [22] invalid argument
...

PHP Fatal error.

The module lacks size checks that causes it to crash, this is not a feature request.

Had a typo in my prev comment: "... Via regular exception, i.e. error".

Since the module is currently crashing with mmap2 failed() that kills the process, I really yhink it is a security issue, even though the sanity check does enforce new (but sane) limitation on the user.

@stas I believe the point here is that an unsanitised user-supplied value is being passed to emalloc, which can be used to crash the process via huge memory allocation.

// zipentry.uncompsize is user-supplied input (a field in the phar structure)
entry.uncompressed_filesize = PHAR_GET_32(zipentry.uncompsize);

// No validation is performed, and value blindly passed to emalloc
sig = (char *) emalloc(entry.uncompressed_filesize);

Bump. Anything new about this ticket?

> 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)

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.

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.

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.

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>

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.