Manifest.read throws an exception with the jar file name passed to the
constructor the manifest was constructed with and not passed to the
call to the read that throws the exception because the jarFilename
variable is not reset after successful construction with read and will
be used by subsequent calls to read if read is called (again) after the
manifest has been constructed. The call to the constructor could be in
a different context than the call to read and the jar file name could
therefore be exposed in an unexpected context. After I first thought it
was just annoying to get an unrelated jar file name in an exception
message I see now a security concern as well.
At first glance and in terms of expectable code changes to the
questionable constructor of Manifest the above mentioned seems to be
overlapping with issue JDK-8216362 but then JDK-8216362 is about the
jar file name missing in an error message when it should be present and
not the other way round. Attached is a patch for JDK-8216362 as it is
described at the moment.
Philipp
On Tue, 2019-01-08 at 13:07 -0500, Sean Mullan wrote:
> In this case, the caller is passing in the filename through the public
> JarFile API so as long as it is not modified it should be ok. The
> concerns I raised previously are situations where the caller did not
> pass in the file or the JDK converts a relative path to an absolute
> path, which could reveal sensitive details about the filesystem.
>
> --Sean
diff -r 7d1efad039a3 src/java.base/share/classes/java/util/jar/Manifest.java
--- a/src/java.base/share/classes/java/util/jar/Manifest.java Mon Jan 07 14:15:00 2019 -0500
+++ b/src/java.base/share/classes/java/util/jar/Manifest.java Wed Jan 09 00:42:04 2019 +0100
@@ -86,7 +86,7 @@
*
* @param is the input stream containing manifest data
* @param jarFilename the name of the corresponding jar archive if available
- * @throws IOException if an I/O error has occured
+ * @throws IOException if an I/O error has occurred
*/
Manifest(InputStream is, String jarFilename) throws IOException {
this(null, is, jarFilename);
@@ -96,9 +96,10 @@
* Constructs a new Manifest from the specified input stream
* and associates it with a JarVerifier.
*/
- Manifest(JarVerifier jv, InputStream is, String jarFilename) throws IOException {
+ Manifest(JarVerifier jv, InputStream is, String jarFilename)
+ throws IOException {
+ this.jarFilename = jarFilename;
read(is);
- this.jarFilename = jarFilename;
this.jv = jv;
}
@@ -315,8 +316,8 @@
if (name == null) {
name = parseName(lbuf, len);
if (name == null) {
- throw new IOException("invalid manifest format"
- + getErrorPosition(jarFilename, lineNumber) + ")");
+ throw new IOException("invalid manifest format ("
+ + getErrorPosition(jarFilename, lineNumber) + ")");
}
if (fis.peek() == ' ') {
// name is wrapped
diff -r 7d1efad039a3 test/jdk/java/util/jar/JarFile/ReportErrorPositionFilename.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/JarFile/ReportErrorPositionFilename.java Wed Jan 09 00:42:04 2019 +0100
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.concurrent.Callable;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.jar.JarOutputStream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * @test
+ * @bug 8216362
+ * @run main/othervm -Djdk.includeInExceptions=foo ReportErrorPositionFilename false
+ * @run main/othervm -Djdk.includeInExceptions=jar ReportErrorPositionFilename true
+ * @summary Verifies that reading a jar file with an invalid manifest
+ * reports its name as part of the exception message if
+ * <code>jdk.includeInExceptions=jar</code>.
+ */ /*
+ * @see Manifest#Manifest(JarVerifier,InputStream,String)
+ * @see Manifest#getErrorPosition
+ */
+public class ReportErrorPositionFilename {
+
+ static final String FILENAME = "Unique-Filename-Expected-In_Msg.jar";
+
+ static final byte[] INVALID_MANIFEST = (
+ "Manifest-Version: 1.0\r\n" +
+ "\r\n" +
+ "Illegal\r\n" +
+ "\r\n").getBytes(UTF_8);
+
+ static void createJarInvalidManifest(String jar) throws IOException {
+ try (OutputStream out = Files.newOutputStream(Paths.get(jar));
+ JarOutputStream jos = new JarOutputStream(out)) {
+ JarEntry je = new JarEntry(JarFile.MANIFEST_NAME);
+ jos.putNextEntry(je);
+ jos.write(INVALID_MANIFEST);
+ jos.closeEntry();
+ }
+ }
+
+ static void test(boolean expectFilenameInErrorMsg, Callable<?> attempt)
+ throws Exception {
+ try {
+ attempt.call();
+ throw new AssertionError("expected exception");
+ } catch (IOException e) {
+ e.printStackTrace(System.out);
+ if (expectFilenameInErrorMsg != e.getMessage().contains(FILENAME)) {
+ throw new AssertionError("file name in error message wrong");
+ }
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ boolean expectFilenameInErrorMsg = Boolean.valueOf(args[0]);
+ createJarInvalidManifest(FILENAME);
+ test(expectFilenameInErrorMsg, () ->
+ new JarFile(FILENAME).getManifest());
+ test(expectFilenameInErrorMsg, () ->
+ new JarFile(FILENAME, true).getManifest());
+ }
+
+}
