resolved conflicts for merge of 6ee07507 to jb-mr1-dev-plus-aosp
Change-Id: Iea67d0b1dd161ea565d31af29f15ee5c1892915e
diff --git a/luni/src/main/java/java/util/zip/ZipEntry.java b/luni/src/main/java/java/util/zip/ZipEntry.java
index 4456640..e9168b5 100644
--- a/luni/src/main/java/java/util/zip/ZipEntry.java
+++ b/luni/src/main/java/java/util/zip/ZipEntry.java
@@ -21,6 +21,7 @@
import java.io.InputStream;
import java.nio.ByteOrder;
import java.nio.charset.Charsets;
+import java.util.Arrays;
import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;
@@ -357,7 +358,13 @@
throw new ZipException("Central Directory Entry not found");
}
- it.seek(10);
+ it.seek(8);
+ int gpbf = it.readShort() & 0xffff;
+
+ if ((gpbf & ZipFile.GPBF_UNSUPPORTED_MASK) != 0) {
+ throw new ZipException("Invalid General Purpose Bit Flag: " + gpbf);
+ }
+
compressionMethod = it.readShort() & 0xffff;
time = it.readShort() & 0xffff;
modDate = it.readShort() & 0xffff;
@@ -377,6 +384,9 @@
byte[] nameBytes = new byte[nameLength];
Streams.readFully(in, nameBytes, 0, nameBytes.length);
+ if (containsNulByte(nameBytes)) {
+ throw new ZipException("Filename contains NUL byte: " + Arrays.toString(nameBytes));
+ }
name = new String(nameBytes, 0, nameBytes.length, Charsets.UTF_8);
// The RI has always assumed UTF-8. (If GPBF_UTF8_FLAG isn't set, the encoding is
@@ -392,4 +402,13 @@
Streams.readFully(in, extra, 0, extraLength);
}
}
+
+ private static boolean containsNulByte(byte[] bytes) {
+ for (byte b : bytes) {
+ if (b == 0) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/luni/src/main/java/java/util/zip/ZipFile.java b/luni/src/main/java/java/util/zip/ZipFile.java
index fbfe254..519459b 100644
--- a/luni/src/main/java/java/util/zip/ZipFile.java
+++ b/luni/src/main/java/java/util/zip/ZipFile.java
@@ -47,6 +47,12 @@
*/
public class ZipFile implements ZipConstants {
/**
+ * General Purpose Bit Flags, Bit 0.
+ * If set, indicates that the file is encrypted.
+ */
+ static final int GPBF_ENCRYPTED_FLAG = 1 << 0;
+
+ /**
* General Purpose Bit Flags, Bit 3.
* If this bit is set, the fields crc-32, compressed
* size and uncompressed size are set to zero in the
@@ -68,6 +74,16 @@
static final int GPBF_UTF8_FLAG = 1 << 11;
/**
+ * Supported General Purpose Bit Flags Mask.
+ * Bit mask of bits not supported.
+ * Note: The only bit that we will enforce at this time
+ * is the encrypted bit. Although other bits are not supported,
+ * we must not enforce them as this could break some legitimate
+ * use cases (See http://b/8617715).
+ */
+ static final int GPBF_UNSUPPORTED_MASK = GPBF_ENCRYPTED_FLAG;
+
+ /**
* Open zip file for reading.
*/
public static final int OPEN_READ = 1;
@@ -242,21 +258,33 @@
RandomAccessFile localRaf = raf;
synchronized (localRaf) {
// We don't know the entry data's start position. All we have is the
- // position of the entry's local header. At position 28 we find the
- // length of the extra data. In some cases this length differs from
- // the one coming in the central header.
- RAFStream rafStream = new RAFStream(localRaf, entry.localHeaderRelOffset + 28);
+ // position of the entry's local header. At position 6 we find the
+ // General Purpose Bit Flag.
+ // http://www.pkware.com/documents/casestudies/APPNOTE.TXT
+ RAFStream rafStream= new RAFStream(localRaf, entry.localHeaderRelOffset + 6);
DataInputStream is = new DataInputStream(rafStream);
- int localExtraLenOrWhatever = Short.reverseBytes(is.readShort()) & 0xffff;
+ int gpbf = Short.reverseBytes(is.readShort()) & 0xffff;
+ if ((gpbf & ZipFile.GPBF_UNSUPPORTED_MASK) != 0) {
+ throw new ZipException("Invalid General Purpose Bit Flag: " + gpbf);
+ }
+
+ // Offset 26 has the file name length, and offset 28 has the extra field length.
+ // These lengths can differ from the ones in the central header.
+ is.skipBytes(18);
+ int fileNameLength = Short.reverseBytes(is.readShort()) & 0xffff;
+ int extraFieldLength = Short.reverseBytes(is.readShort()) & 0xffff;
is.close();
- // Skip the name and this "extra" data or whatever it is:
- rafStream.skip(entry.nameLength + localExtraLenOrWhatever);
- rafStream.length = rafStream.offset + entry.compressedSize;
+ // Skip the variable-size file name and extra field data.
+ rafStream.skip(fileNameLength + extraFieldLength);
+
+ // The compressed or stored file data follows immediately after.
if (entry.compressionMethod == ZipEntry.DEFLATED) {
- int bufSize = Math.max(1024, (int)Math.min(entry.getSize(), 65535L));
+ rafStream.length = rafStream.offset + entry.compressedSize;
+ int bufSize = Math.max(1024, (int) Math.min(entry.getSize(), 65535L));
return new ZipInflaterInputStream(rafStream, new Inflater(true), bufSize, entry);
} else {
+ rafStream.length = rafStream.offset + entry.size;
return rafStream;
}
}
diff --git a/luni/src/main/java/java/util/zip/ZipInputStream.java b/luni/src/main/java/java/util/zip/ZipInputStream.java
index 97aa350..d46e2dd 100644
--- a/luni/src/main/java/java/util/zip/ZipInputStream.java
+++ b/luni/src/main/java/java/util/zip/ZipInputStream.java
@@ -237,6 +237,10 @@
throw new ZipException("Cannot read local header version " + version);
}
int flags = peekShort(LOCFLG - LOCVER);
+ if ((flags & ZipFile.GPBF_UNSUPPORTED_MASK) != 0) {
+ throw new ZipException("Invalid General Purpose Bit Flag: " + flags);
+ }
+
hasDD = ((flags & ZipFile.GPBF_DATA_DESCRIPTOR_FLAG) != 0);
int ceLastModifiedTime = peekShort(LOCTIM - LOCVER);
int ceLastModifiedDate = peekShort(LOCTIM - LOCVER + 2);
diff --git a/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java b/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java
index 49dc050..45bd860 100644
--- a/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java
+++ b/luni/src/test/java/libcore/java/util/zip/ZipFileTest.java
@@ -33,7 +33,6 @@
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;
import junit.framework.TestCase;
-import libcore.io.IoUtils;
public final class ZipFileTest extends TestCase {
/**
@@ -57,7 +56,7 @@
zipFile.close();
}
- private static void replaceBytes(byte[] original, byte[] replacement, byte[] buffer) {
+ private static void replaceBytes(byte[] buffer, byte[] original, byte[] replacement) {
// Gotcha here: original and replacement must be the same length
assertEquals(original.length, replacement.length);
boolean found;
@@ -80,37 +79,38 @@
}
}
+ private static void writeBytes(File f, byte[] bytes) throws IOException {
+ FileOutputStream out = new FileOutputStream(f);
+ out.write(bytes);
+ out.close();
+ }
+
/**
* Make sure we don't fail silently for duplicate entries.
* b/8219321
*/
- public void testDuplicateEntries() throws IOException {
- String entryName = "test_file_name1";
- String tmpName = "test_file_name2";
+ public void testDuplicateEntries() throws Exception {
+ String name1 = "test_file_name1";
+ String name2 = "test_file_name2";
- // create the template data
- ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();
- ZipOutputStream out = new ZipOutputStream(bytesOut);
- ZipEntry ze1 = new ZipEntry(tmpName);
- out.putNextEntry(ze1);
+ // Create the good zip file.
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ ZipOutputStream out = new ZipOutputStream(baos);
+ out.putNextEntry(new ZipEntry(name2));
out.closeEntry();
- ZipEntry ze2 = new ZipEntry(entryName);
- out.putNextEntry(ze2);
+ out.putNextEntry(new ZipEntry(name1));
out.closeEntry();
out.close();
- // replace the bytes we don't like
- byte[] buf = bytesOut.toByteArray();
- replaceBytes(tmpName.getBytes(), entryName.getBytes(), buf);
+ // Rewrite one of the filenames.
+ byte[] buffer = baos.toByteArray();
+ replaceBytes(buffer, name2.getBytes(), name1.getBytes());
- // write the result to a file
- File badZip = File.createTempFile("badzip", "zip");
- badZip.deleteOnExit();
- FileOutputStream outstream = new FileOutputStream(badZip);
- outstream.write(buf);
- outstream.close();
+ // Write the result to a file.
+ File badZip = createTemporaryZipFile();
+ writeBytes(badZip, buffer);
- // see if we can still handle it
+ // Check that we refuse to load the modified file.
try {
ZipFile bad = new ZipFile(badZip);
fail();
@@ -395,6 +395,55 @@
out.close();
}
+ // https://code.google.com/p/android/issues/detail?id=58465
+ public void test_NUL_in_filename() throws Exception {
+ File file = createTemporaryZipFile();
+
+ // We allow creation of a ZipEntry whose name contains a NUL byte,
+ // mainly because it's not likely to happen by accident and it's useful for testing.
+ ZipOutputStream out = createZipOutputStream(file);
+ out.putNextEntry(new ZipEntry("hello"));
+ out.putNextEntry(new ZipEntry("hello\u0000"));
+ out.close();
+
+ // But you can't open a ZIP file containing such an entry, because we reject it
+ // when we find it in the central directory.
+ try {
+ ZipFile zipFile = new ZipFile(file);
+ fail();
+ } catch (ZipException expected) {
+ }
+ }
+
+ public void testNameLengthChecks() throws IOException {
+ // Is entry name length checking done on bytes or characters?
+ // Really it should be bytes, but the RI only checks characters at construction time.
+ // Android does the same, because it's cheap...
+ try {
+ new ZipEntry((String) null);
+ fail();
+ } catch (NullPointerException expected) {
+ }
+ new ZipEntry(makeString(0xffff, "a"));
+ try {
+ new ZipEntry(makeString(0xffff + 1, "a"));
+ fail();
+ } catch (IllegalArgumentException expected) {
+ }
+
+ // ...but Android won't let you create a zip file with a truncated name.
+ ZipOutputStream out = createZipOutputStream(createTemporaryZipFile());
+ ZipEntry ze = new ZipEntry(makeString(0xffff, "\u0666"));
+ try {
+ out.putNextEntry(ze);
+ fail(); // The RI fails this test; it just checks the character count at construction time.
+ } catch (IllegalArgumentException expected) {
+ }
+ out.closeEntry();
+ out.putNextEntry(new ZipEntry("okay")); // ZipOutputStream.close throws if you add nothing!
+ out.close();
+ }
+
public void testCrc() throws IOException {
ZipEntry ze = new ZipEntry("test");
ze.setMethod(ZipEntry.STORED);