Last modified: 2014-11-17 09:21:20 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T26230, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 24230 - Implement JAR detection
Implement JAR detection
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Uploading (Other open bugs)
1.17.x
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: 2089 17858 16241 20429 21594 23829
  Show dependency treegraph
 
Reported: 2010-07-02 12:17 UTC by Derk-Jan Hartman
Modified: 2014-11-17 09:21 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Derk-Jan Hartman 2010-07-02 12:17:37 UTC
We should find a reliable JAR detection routine, so that we can block JAR files, instead of having to whitelist all the different zip based fileformats.

Solution:
* Do a simple ZIP detection like we have now:
* Read with ZipArchive http://php.net/manual/en/ref.zip.php
* Traverse and look with zip_entry_name() for files with:
** MANIFEST.MF
** .class or .java or .jar

I'm not sure if this works well enough to plug the GIFAR hole however, because we don't really know how Java detects if a zip == a jar. Will have to be verified somehow.
Comment 1 Max Semenik 2010-07-02 13:42:12 UTC
The relevant detection code is at 
http://www.java2s.com/Open-Source/Java-Document/6.0-JDK-Core/Collections-Jar-Zip-Logging-regex/java/util/jar/JarFile.java.htm

Shortly, it checks for META-INF/MANIFEST.MF case-insensitive. To be completely sure, any file containing directory META-INF should be considered potentially dangerous.
Comment 2 JeLuF 2010-07-02 21:36:55 UTC
The EPUB file format also has a META-INF directory.
Comment 3 Marcin Cieślak 2011-02-07 00:28:52 UTC
To be sucessfully execute, JAR file needs to have a "Main-Class" entry in the META-INF/MANIFEST.MF file:

Here is a test using the abovementioned class using Jython (http://www.jython.org/):

$ jython                                          
Jython 2.5.1 (Release_2_5_1:6813, Sep 26 2009, 13:47:54) 
[Java HotSpot(TM) 64-Bit Server VM (Sun Microsystems Inc.)] on java1.6.0_03-p4
Type "help", "copyright", "credits" or "license" for more information.
>>> import java
>>> y = java.util.jar.JarFile("/usr/local/share/java/apache-ant/lib/ant.jar") 
>>> print y.manifest.mainAttributes.getValue("Main-Class")
org.apache.tools.ant.Main
>>> y = java.util.jar.JarFile("/usr/local/share/java/classes/log4j.jar")     
>>> print y.manifest.mainAttributes.getValue("Main-Class")              
None

So "ant.jar" is "executable" with while log4j.jar is not. 

Maybe checking for "Main-Class:" string is better, since zip_open() does not really detect embedded JAR as a ZIP file (checked with http://quarkmitsauce.files.wordpress.com/2008/08/gifar.gif). 

This gif file has no zip entries according to PHP zip_entry_name, but Java still detects the manifest:

$ java -jar gifar.gif 
Exception in thread "main" java.lang.NoClassDefFoundError: gifar/Main

We need a reasonable middle ground between a crude check of sequence of bytes and canonical unpacking of the ZIP file.
Comment 4 Tim Starling 2011-02-07 01:21:21 UTC
(In reply to comment #1)
> The relevant detection code is at 
> http://www.java2s.com/Open-Source/Java-Document/6.0-JDK-Core/Collections-Jar-Zip-Logging-regex/java/util/jar/JarFile.java.htm
> 
> Shortly, it checks for META-INF/MANIFEST.MF case-insensitive. To be completely
> sure, any file containing directory META-INF should be considered potentially
> dangerous.

It's not necessary to have a META-INF directory. Here is page with an applet which doesn't have one, it works just fine:

http://noc.wikimedia.org/~tstarling/odjar/

java.util.jar is not used to open JAR files during startup. The class files for it themselves inside a JAR file. Instead, the native C code is used directly:

<http://hg.openjdk.java.net/jdk7/tl/jdk/file/a06412e13bf7/src/share/native/java/util/zip/>

(In reply to comment #3)
> $ java -jar gifar.gif 
> Exception in thread "main" java.lang.NoClassDefFoundError: gifar/Main

The <applet> tag can contain a "code" attribute which specifies which class to run. In this case, no Main-Class entry (and indeed no manifest at all) is required.

To check if a zip file is safe, you have to check whether there are any files in it with a ".class" extension. To do this, you have to scan the central directory, with a zip reader that supports ZIP64 including the central directory compression feature. Doing this in pure PHP is possible, using zlib for decompression, but note that the PEAR Archive_Zip library is not good enough because it does not support ZIP64.

(In reply to comment #0)
> * Read with ZipArchive http://php.net/manual/en/ref.zip.php

That extension has a big ugly "unmaintained" warning on it, the PECL extension is recommended instead:

http://www.php.net/manual/en/zip.installation.php

The zip extension is not enabled by default, and so most installations do not have access to it. The zlib extension is enabled by default, that's why I think it's the best solution. Java only supports zlib for decompression, so there's no need to support any other decompression method to check for safety in Java.
Comment 5 Tim Starling 2011-02-07 01:29:29 UTC
Correction: zlib is not enabled by default when you compile PHP from source. However it is enabled in the Windows builds, and in the Debian packages.
Comment 6 Bryan Tong Minh 2011-02-07 09:31:30 UTC
(In reply to comment #5)
> Correction: zlib is not enabled by default when you compile PHP from source.
> However it is enabled in the Windows builds, and in the Debian packages.

That's good enough if we keep on rejecting zip files by default when we're not able to detect them
Comment 7 Tim Starling 2011-02-25 04:58:04 UTC
Done in r82783. It turns out that there is no such thing as the ZIP64 central directory compression feature (that was a misreading of the spec), and ZIP64 support is mostly counterproductive. But I only worked that out after the reader was almost finished, so I figured I may as well finish it and commit it.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links