Avoid override target symlink by standard file in AbstractUnArchiver

codehaus-plexus · Jul 25, 2023 · 5475983 · 5475983
1 parent 18a6485
commit 5475983
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 5 deletions.
diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
@@ -20,7 +20,6 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -35,11 +34,12 @@
 import org.codehaus.plexus.components.io.fileselectors.FileSelector;
 import org.codehaus.plexus.components.io.resources.PlexusIoResource;
 import org.codehaus.plexus.util.FileUtils;
-import org.codehaus.plexus.util.IOUtil;
 import org.codehaus.plexus.util.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
 // TODO there should really be constructors which take the source file.
 
 /**
@@ -301,6 +301,11 @@ protected void extractFile(
             throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
         }
 
+        // don't allow override target symlink by standard file
+        if (StringUtils.isEmpty(symlinkDestination) && Files.isSymbolicLink(canonicalDestPath)) {
+            throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
+        }
+
         try {
             if (!shouldExtractEntry(dir, targetFileName, entryName, entryDate)) {
                 return;
@@ -317,9 +322,7 @@ protected void extractFile(
             } else if (isDirectory) {
                 targetFileName.mkdirs();
             } else {
-                try (OutputStream out = Files.newOutputStream(targetFileName.toPath())) {
-                    IOUtil.copy(compressedInputStream, out);
-                }
+                Files.copy(compressedInputStream, targetFileName.toPath(), REPLACE_EXISTING);
             }
 
             targetFileName.setLastModified(entryDate.getTime());

diff --git a/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java b/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java
@@ -62,6 +62,8 @@ void testSymlinkTar() throws Exception {
         unarchiver.setSourceFile(archiveFile);
         unarchiver.setDestFile(output);
         unarchiver.extract();
+        // second unpacking should also work
+        unarchiver.extract();
     }
 
     @Test
@@ -81,6 +83,8 @@ void testSymlinkZip() throws Exception {
         unarchiver.setSourceFile(archiveFile);
         unarchiver.setDestFile(output);
         unarchiver.extract();
+        // second unpacking should also work
+        unarchiver.extract();
     }
 
     @Test

diff --git a/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java
@@ -85,6 +85,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -809,6 +810,17 @@ void testFixedEntryModificationTime() throws IOException {
         }
     }
 
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    void testNonExistingSymlink() throws Exception {
+        File zipFile = new File("src/test/resources/symlinks/non_existing_symlink.zip");
+        ZipUnArchiver unArchiver = getZipUnArchiver(zipFile);
+        String tmpdir = Files.createTempDirectory("tmpe_extract").toFile().getAbsolutePath();
+        unArchiver.setDestDirectory(new File(tmpdir));
+        ArchiverException exception = assertThrows(ArchiverException.class, unArchiver::extract);
+        assertEquals("Entry is outside of the target directory (entry1)", exception.getMessage());
+    }
+
     /**
      * Takes a timestamp, turns it into a textual representation based on GMT, then translated it into a timestamp in
      * local timezone. This makes the test independent of the current TimeZone. The reason this is necessary is:

diff --git a/src/test/resources/symlinks/non_existing_symlink.zip b/src/test/resources/symlinks/non_existing_symlink.zip
diff --git a/src/test/resources/symlinks/regen.sh b/src/test/resources/symlinks/regen.sh
@@ -4,3 +4,13 @@ cd src
 zip --symlinks ../symlinks.zip file* targetDir sym*
 tar -cvf ../symlinks.tar file* targetDir sym*
 
+cd ..
+rm non_existing_symlink.zip
+mkdir non_existing_symlink
+cd non_existing_symlink
+ln -s /tmp/target entry1
+echo -ne 'content' > entry2
+zip  --symlinks ../non_existing_symlink.zip entry1 entry2
+cd ..
+rm -rf non_existing_symlink
+LC_ALL=C sed  -i '' 's/entry2/entry1/' non_existing_symlink.zip