Continuing our discussion #6173, I managed to replicate this issue. When using a lesser ZIP compression setting, some <?php tags will not be encoded in the archive, thus they will appear as plain text.
After applying the patch the installation worked correctly.

Great! We now have two successful tests. Can we please set this RTC, @wilsonge ?

Set RTC

Thank you @nikosdion for the fast fix!

hello friends, this should get fixed asap, because right now if the zip's binary contains the php tags, it cannot be installed...

secondly it's not a good idea to load file buffer and do the comparison from within php because you can get memory leak very easily. also by loading the file into php's memory you never know if somebody find a CVE in future which would be able to execute the binary code loaded... you cannot be sure it's unloaded even with unset($data); as this relies purely on php's GC.

ok, the zip package can include the <?php contents when there is a php file that contains nothing else then the starting php tag <?php
which normally should be okay with joomla because it requires defined('_JEXEC') or die; and thus the file may be subject for suspicion

@stAn47 As you saw I already fixed it in this PR and it was already merged 4 hours ago by @Kubik-Rubik. As a result this issue is now fixed and the fix will appear in Joomla! 3.4.1.

Regarding the rest of your comments. Ideally, you should filter uploads at the server level, using a security module running inside Apache (or whatever server you use), long before PHP loads. The UploadShield implementation is a last ditch effort to prevent maliciously crafted files from being uploaded through a Joomla! extension.

As for the possibility of a memory leak... It's file_get_contents(). It's never been used as an attack vector like you describe and even if it did develop a memory leak it would be patched in a heartbeat considering that it is a fundamental PHP function. I also dispute your reasoning because it's so far fetched it can only be described as irrational. Let me give you an analogy of what you're saying. We should abolish seat belts in cars because cars use petrol, a highly flammable liquid. What if at some point cars develop an issue where the petrol tank catches fire and the occupants cannot exit it –and die in the blazing inferno– because they couldn't unfasten their seat belt? I hope you understand why cars still have seat belts. The hypothetical die-in-an-inferno scenario is extremely unlikely compared to the daily occurring die-in-a-car-crash-without-a-seat-belt scenario. The same applies for our sites, as I'll tell you below.

As for your last comment, I am not sure if you understand the problem and the solution. It's the most basic way to hack a site (and one that does happen out there all the time). If I am a really bad person who uploads a C99 script, let's call it die.png.php, through a badly written extension's front-end to media/com_idiot on your site I can access it over the web, through my browser, as http://www.example.com/media/com_idiot/die.png.php and hack your site. Do I have to spell out how a C99 script can hack your site? Google it. My C99 script doesn't need to have defined('_JEXEC') or die as it doesn't load through Joomla!. UploadShield is designed to protect regular Joomla! users against this attack vector which is NOT present in Joomla! itself but in lower quality third party extensions. That's a very real threat, unlike your entirely hypothetical memory leak leading to arbitrary binary code execution.

Hello Nicholas, i do understand the importance of having such a protection, but it would be ideal that if there is some protection that can potentionally limit the previous functionality (for watever reason), the user would at least get the message which would tell him which protection level had refused his file. A jpg file can easily legitimately contain php starting short tags. Or maybe ideally it could be disabled within the configuration for at least special cases.

Regarding the buffer,maybe it could at least check if it has enough memory to read it. (php upload size 320mb and php memory limit only 128mb), strpos and stripos may also take a huge amount of memory.

I understand your concerns and I had the same qualms about short open tags back in 2010 when I first wrote UploadShield. After 5 years and thousands of my clients using UploadShield on tens of thousands of sites I can tell you that short open tags are virtually impossible to find in JPG or any other kind of media files.

However, I do share your concerns which is why JFile::upload() has the fourth and fifth parameters and why JInputFiles allows you to ignore filtering by using the 'raw' filter. Basically, if a developer wants to disable this feature, they can do so very easily. The whole idea was to protect users against developers who don't know they should check the file extensions and contents and let users of their extensions do unsafe things. All other developers can disable or customise the scanning depending on their use case.

As for the message, it is logged. It doesn't appear in the browser output for a reason: you don't want to tip off malicious users that their file was rejected because of the contents.

As for memory leaks with large files you are ignoring some very basic PHP realities:

• The maximum upload size AND the maximum post size are typically an order of magnitude (or more) smaller than the PHP memory_limit. Increasing these limits to several hundred megabytes more than your PHP memory is unwise. Forget memory leaks. If you allow people to upload hundreds of Mb at a time you allow them to consume a huge amount of bandwidth and TCP/IP stack resources, all the while making sure that they cannot be stopped by your DDoS protection service. I can think of easier ways to open a huge DDoS backdoor to your site, but this one would work too...

• PHP will try to allocate the memory for a variable before using it. It's not C where an unchecked pointer can wreck havoc. If you try loading a 320Mb file into a site with 128Mb of PHP memory you'll get a memory outage error and that's the end of it. Same applies to all other string functions. However, one would have to first override the previous safeguard which is, frankly, moronic. But even in this case you don't get a memory leak, just a fatal error. If for any reason a developer wants to do that –after all, some people do like doing stupid things like juggling chainsaws which can chop their hands off and allowing ginormous single-part uploads which can DDoS their server– they could just turn off file checking and make sure they never use JInputFiles.

So, yeah, I understand your concerns and they'd make check if there were no upload limits and PHP was using pointers instead of pre-allocated memory segments and didn't do memory checks. You can make the first condition true, the other two not. It would take an exceptional combination of server engineer idiocy and two extremely grave bugs in PHP to bring your attack scenario into the realm of plausibility. Even then it would take another grave bug in PHP to make the arbitrary code executable. Even then it would take a platform with no executable bit support and lack of address space randomisation. That's too far fetched. If someone manages to detect and exploit this kind of extremely unlikely perfect storm they DESERVE to hack your site.