Page MenuHomePhabricator

Implement JAR detection
Closed, ResolvedPublic

Description

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.


Version: 1.17.x
Severity: enhancement
See Also:

Details

Reference
bz24230

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:10 PM
bzimport set Reference to bz24230.
bzimport added a subscriber: Unknown Object (MLST).

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.

jeluf wrote:

The EPUB file format also has a META-INF directory.

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.

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

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.

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.

Bryan.TongMinh wrote:

(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

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.

Gilles raised the priority of this task from Medium to Unbreak Now!.Dec 4 2014, 10:11 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to Medium.Dec 4 2014, 11:20 AM