diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java index e8a6a2e2d..6588f822e 100644 --- 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 index aeb607871..b118e81bf 100644 --- 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 index 97af36d46..9d69f45d2 100644 --- 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 new file mode 100644 index 000000000..c390c8ea5 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 f3c3a9d73..e6a532e95 100755 --- 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 \ No newline at end of file