fix edge case vulnerability detected by @DidierLoiseau

While this allows a path traversal attack it can only be exploited in a special edge case.
apache · Jun 15, 2018 · a080293 · a080293
1 parent ba12419
commit a080293
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 1 deletion.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -44,6 +44,12 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
     <release version="1.18" date="not released, yet"
              description="Release 1.18">
+      <action type="fix" date="2018-06-15" due-to="DidierLoiseau">
+        The example Expander class has been vulnerable to a path
+        traversal in the edge case that happens when the target
+        directory has a sibling directory and the name of the target
+        directory is a prefix of the sibling directory's name.
+      </action>
     </release>
     <release version="1.17" date="2018-06-03"
              description="Release 1.17">

diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
@@ -238,7 +238,7 @@ private boolean prefersSeekableByteChannel(String format) {
 
     private void expand(ArchiveEntrySupplier supplier, EntryWriter writer, File targetDirectory)
         throws IOException {
-        String targetDirPath = targetDirectory.getCanonicalPath();
+        String targetDirPath = targetDirectory.getCanonicalPath() + File.separatorChar;
         ArchiveEntry nextEntry = supplier.getNextReadableEntry();
         while (nextEntry != null) {
             File f = new File(targetDirectory, nextEntry.getName());

diff --git a/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java b/src/test/java/org/apache/commons/compress/archivers/examples/ExpanderTest.java
@@ -43,6 +43,7 @@
 import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.apache.commons.compress.utils.IOUtils;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -130,6 +131,26 @@ public void fileCantEscapeDoubleDotPath() throws IOException, ArchiveException {
         }
     }
 
+    @Test
+    public void fileCantEscapeDoubleDotPathWithSimilarSibling() throws IOException, ArchiveException {
+        String sibling = resultDir.getName() + "x";
+        File s = new File(resultDir.getParentFile(), sibling);
+        Assume.assumeFalse(s.exists());
+        s.mkdirs();
+        Assume.assumeTrue(s.exists());
+        s.deleteOnExit();
+        try {
+            thrown.expect(IOException.class);
+            thrown.expectMessage("expanding ../" + sibling + "/a would create file outside of");
+            setupZip("../" + sibling + "/a");
+            try (ZipFile f = new ZipFile(archive)) {
+                new Expander().expand(f, resultDir);
+            }
+        } finally {
+            tryHardToDelete(s);
+        }
+    }
+
     private void setup7z() throws IOException, ArchiveException {
         archive = new File(dir, "test.7z");
         File dummy = new File(dir, "x");