Skip to content

Commit 5475983

Browse files
uriyay-jfrogslawekjaranowski
authored andcommitted
Avoid override target symlink by standard file in AbstractUnArchiver
1 parent 18a6485 commit 5475983

File tree

5 files changed

+34
-5
lines changed

5 files changed

+34
-5
lines changed

src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java

+8-5
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.io.FileNotFoundException;
2121
import java.io.IOException;
2222
import java.io.InputStream;
23-
import java.io.OutputStream;
2423
import java.nio.file.Files;
2524
import java.nio.file.Path;
2625
import java.util.ArrayList;
@@ -35,11 +34,12 @@
3534
import org.codehaus.plexus.components.io.fileselectors.FileSelector;
3635
import org.codehaus.plexus.components.io.resources.PlexusIoResource;
3736
import org.codehaus.plexus.util.FileUtils;
38-
import org.codehaus.plexus.util.IOUtil;
3937
import org.codehaus.plexus.util.StringUtils;
4038
import org.slf4j.Logger;
4139
import org.slf4j.LoggerFactory;
4240

41+
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
42+
4343
// TODO there should really be constructors which take the source file.
4444

4545
/**
@@ -301,6 +301,11 @@ protected void extractFile(
301301
throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
302302
}
303303

304+
// don't allow override target symlink by standard file
305+
if (StringUtils.isEmpty(symlinkDestination) && Files.isSymbolicLink(canonicalDestPath)) {
306+
throw new ArchiverException("Entry is outside of the target directory (" + entryName + ")");
307+
}
308+
304309
try {
305310
if (!shouldExtractEntry(dir, targetFileName, entryName, entryDate)) {
306311
return;
@@ -317,9 +322,7 @@ protected void extractFile(
317322
} else if (isDirectory) {
318323
targetFileName.mkdirs();
319324
} else {
320-
try (OutputStream out = Files.newOutputStream(targetFileName.toPath())) {
321-
IOUtil.copy(compressedInputStream, out);
322-
}
325+
Files.copy(compressedInputStream, targetFileName.toPath(), REPLACE_EXISTING);
323326
}
324327

325328
targetFileName.setLastModified(entryDate.getTime());

src/test/java/org/codehaus/plexus/archiver/SymlinkTest.java

+4
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ void testSymlinkTar() throws Exception {
6262
unarchiver.setSourceFile(archiveFile);
6363
unarchiver.setDestFile(output);
6464
unarchiver.extract();
65+
// second unpacking should also work
66+
unarchiver.extract();
6567
}
6668

6769
@Test
@@ -81,6 +83,8 @@ void testSymlinkZip() throws Exception {
8183
unarchiver.setSourceFile(archiveFile);
8284
unarchiver.setDestFile(output);
8385
unarchiver.extract();
86+
// second unpacking should also work
87+
unarchiver.extract();
8488
}
8589

8690
@Test

src/test/java/org/codehaus/plexus/archiver/zip/ZipArchiverTest.java

+12
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import static org.junit.jupiter.api.Assertions.assertEquals;
8686
import static org.junit.jupiter.api.Assertions.assertFalse;
8787
import static org.junit.jupiter.api.Assertions.assertNotNull;
88+
import static org.junit.jupiter.api.Assertions.assertThrows;
8889
import static org.junit.jupiter.api.Assertions.assertTrue;
8990
import static org.junit.jupiter.api.Assertions.fail;
9091

@@ -809,6 +810,17 @@ void testFixedEntryModificationTime() throws IOException {
809810
}
810811
}
811812

813+
@Test
814+
@DisabledOnOs(OS.WINDOWS)
815+
void testNonExistingSymlink() throws Exception {
816+
File zipFile = new File("src/test/resources/symlinks/non_existing_symlink.zip");
817+
ZipUnArchiver unArchiver = getZipUnArchiver(zipFile);
818+
String tmpdir = Files.createTempDirectory("tmpe_extract").toFile().getAbsolutePath();
819+
unArchiver.setDestDirectory(new File(tmpdir));
820+
ArchiverException exception = assertThrows(ArchiverException.class, unArchiver::extract);
821+
assertEquals("Entry is outside of the target directory (entry1)", exception.getMessage());
822+
}
823+
812824
/**
813825
* Takes a timestamp, turns it into a textual representation based on GMT, then translated it into a timestamp in
814826
* local timezone. This makes the test independent of the current TimeZone. The reason this is necessary is:
Binary file not shown.

src/test/resources/symlinks/regen.sh

+10
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,13 @@ cd src
44
zip --symlinks ../symlinks.zip file* targetDir sym*
55
tar -cvf ../symlinks.tar file* targetDir sym*
66

7+
cd ..
8+
rm non_existing_symlink.zip
9+
mkdir non_existing_symlink
10+
cd non_existing_symlink
11+
ln -s /tmp/target entry1
12+
echo -ne 'content' > entry2
13+
zip --symlinks ../non_existing_symlink.zip entry1 entry2
14+
cd ..
15+
rm -rf non_existing_symlink
16+
LC_ALL=C sed -i '' 's/entry2/entry1/' non_existing_symlink.zip

0 commit comments

Comments
 (0)