CVE-2023-37460: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Plexis Archiver is a collection of Plexus components to create archives or extract archives to a directory with a unified `Archiver`/`UnArchiver` API. Prior to version 4.8.0, using AbstractUnArchiver for extracting an archive might lead to an arbitrary file creation and possibly remote code execution. When extracting an archive with an entry that already exists in the destination directory as a symbolic link whose target does not exist - the `resolveFile()` function will return the symlink's source instead of its target, which will pass the verification that ensures the file will not be extracted outside of the destination directory. Later `Files.newOutputStream()`, that follows symlinks by default,  will actually write the entry's content to the symlink's target. Whoever uses plexus archiver to extract an untrusted archive is vulnerable to an arbitrary file creation and possibly remote code execution. Version 4.8.0 contains a patch for this issue.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.8
Published: 2023-07-25T20:15Z
Repository: https://github.com/codehaus-plexus/plexus-archiver
Language: Java
Files changed: src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java, src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java, src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java, src/test/resources/symlinks/regen.sh
Commit date: 2023-07-25 08:43:46 +0200
Commit message: Avoid override target symlink by standard file in AbstractUnArchiver

Diff: diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
index e8a6a2e2..6588f822 100644
--- a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
+++ b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
@@ -18,11 +18,10 @@
 
 import java.io.File;
 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;
 import java.util.Date;
 import java.util.List;
@@ -33,15 +32,16 @@
 import org.codehaus.plexus.components.io.attributes.SymlinkUtils;
 import org.codehaus.plexus.components.io.filemappers.FileMapper;
 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.
 
 /**
  * @author <a href="mailto:evenisse@codehaus.org">Emmanuel Venisse</a>
  */
@@ -299,10 +299,15 @@ protected void extractFile(
 
         if (!canonicalDestPath.startsWith(canonicalDirPath)) {
             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;
             }
 
@@ -315,13 +320,11 @@ protected void extractFile(
             if (!StringUtils.isEmpty(symlinkDestination)) {
                 SymlinkUtils.createSymbolicLink(targetFileName, new File(symlinkDestination));
             } 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());
 
             if (!isIgnorePermissions() && mode != null && !isDirectory) {
diff --git a/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java b/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java
index aeb60787..b118e81b 100644
--- a/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java
+++ b/src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java
@@ -60,10 +60,12 @@ void testSymlinkTar() throws Exception {
         output.mkdirs();
         TarUnArchiver unarchiver = (TarUnArchiver) lookup(UnArchiver.class, "tar");
         unarchiver.setSourceFile(archiveFile);
         unarchiver.setDestFile(output);
         unarchiver.extract();
+        // second unpacking should also work
+        unarchiver.extract();
     }
 
     @Test
     void testSymlinkZip() throws Exception {
         ZipArchiver archiver = (ZipArchiver) lookup(Archiver.class, "zip");
@@ -79,10 +81,12 @@ void testSymlinkZip() throws Exception {
         output.mkdirs();
         ZipUnArchiver unarchiver = (ZipUnArchiver) lookup(UnArchiver.class, "zip");
         unarchiver.setSourceFile(archiveFile);
         unarchiver.setDestFile(output);
         unarchiver.extract();
+        // second unpacking should also work
+        unarchiver.extract();
     }
 
     @Test
     @DisabledOnOs(OS.WINDOWS)
     void testSymlinkDirArchiver() throws Exception {
diff --git a/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java
index 97af36d4..9d69f45d 100644
--- a/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java
+++ b/src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java
@@ -83,10 +83,11 @@
 import org.junit.jupiter.api.io.TempDir;
 
 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;
 
 /**
  * @author Emmanuel Venisse
@@ -807,10 +808,21 @@ void testFixedEntryModificationTime() throws IOException {
             assertEquals(almostMinDosTime, zf.getEntry("file-with-odd-time.txt").getTime());
             assertEquals(almostMinDosTime, zf.getEntry("foo/").getTime());
         }
     }
 
+    @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:
      * <ul>
      * <li>ZIP file format does not take timezone into account.</li>
diff --git a/src/test/resources/symlinks/non_existing_symlink.zip b/src/test/resources/symlinks/non_existing_symlink.zip
new file mode 100644
index 00000000..c390c8ea
Binary files /dev/null and b/src/test/resources/symlinks/non_existing_symlink.zip differ
diff --git a/src/test/resources/symlinks/regen.sh b/src/test/resources/symlinks/regen.sh
index f3c3a9d7..e6a532e9 100755
--- a/src/test/resources/symlinks/regen.sh
+++ b/src/test/resources/symlinks/regen.sh
@@ -2,5 +2,15 @@ rm symlinks.zip
 rm symlinks.tar
 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
\ No newline at end of file

Vulnerable code: import java.io.OutputStream;
import org.codehaus.plexus.util.IOUtil;
                try (OutputStream out = Files.newOutputStream(targetFileName.toPath())) {
                    IOUtil.copy(compressedInputStream, out);
                }
Fixed code: import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;

        // 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 + ")");
        }

                Files.copy(compressedInputStream, targetFileName.toPath(), REPLACE_EXISTING);
        // second unpacking should also work
        unarchiver.extract();
        // second unpacking should also work
        unarchiver.extract();
import static org.junit.jupiter.api.Assertions.assertThrows;
    @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());
    }

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