From a1be74792662726e4c8d051a690b5ee13b7dd2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armin=20H=C3=A4berling?= Date: Thu, 5 Dec 2019 20:11:38 +0100 Subject: [PATCH 1/3] Add unit tests for DefaultArchiveExtractor * test extraction of malicious tar files * test extraction of a tar file into path with a symlink (this is a test for the fix in commit 6175368) --- .../lib/DefaultArchiveExtractorTest.java | 68 ++++++++++++++++++ .../src/test/resources/bad.tgz | Bin 0 -> 165 bytes .../src/test/resources/good.tgz | Bin 0 -> 146 bytes 3 files changed, 68 insertions(+) create mode 100644 frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java create mode 100644 frontend-plugin-core/src/test/resources/bad.tgz create mode 100644 frontend-plugin-core/src/test/resources/good.tgz diff --git a/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java b/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java new file mode 100644 index 000000000..c50d8e777 --- /dev/null +++ b/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java @@ -0,0 +1,68 @@ +package com.github.eirslett.maven.plugins.frontend.lib; + +import org.hamcrest.CoreMatchers; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; + +public class DefaultArchiveExtractorTest { + + private final String BAD_TAR = "src/test/resources/bad.tgz"; + private final String GOOD_TAR = "src/test/resources/good.tgz"; + + @Rule + public final TemporaryFolder temp = new TemporaryFolder(); + + private ArchiveExtractor extractor; + + @Before + public void setup() { + extractor = new DefaultArchiveExtractor(); + } + + @Test + public void extractGoodTarFile() throws Exception { + File destination = temp.newFolder("destination"); + extractor.extract(GOOD_TAR, destination.getPath()); + } + + @Test + public void extractGoodTarFileSymlink() throws Exception { + File destination = temp.newFolder("destination"); + Path link = createSymlinkOrSkipTest(temp.getRoot().toPath().resolve("link"), destination.toPath()); + extractor.extract(GOOD_TAR, link.toString()); + } + + @Test(expected = ArchiveExtractionException.class) + public void extractBadTarFile() throws Exception { + File destination = temp.newFolder("destination"); + extractor.extract(BAD_TAR, destination.getPath()); + } + + @Test(expected = ArchiveExtractionException.class) + public void extractBadTarFileSymlink() throws Exception { + File destination = temp.newFolder("destination"); + Path link = createSymlinkOrSkipTest(temp.getRoot().toPath().resolve("link"), destination.toPath()); + extractor.extract(BAD_TAR, link.toString()); + } + + private Path createSymlinkOrSkipTest(Path link, Path target) throws IOException { + try { + return Files.createSymbolicLink(link, target); + } catch (UnsupportedOperationException e) { + assumeTrue("symlinks not supported", false); + return null; + } + } +} diff --git a/frontend-plugin-core/src/test/resources/bad.tgz b/frontend-plugin-core/src/test/resources/bad.tgz new file mode 100644 index 0000000000000000000000000000000000000000..f287c1aae88218449f29b8a49d7bf7e04cda8caa GIT binary patch literal 165 zcmb2|=HSrqdl}2XoRpZNSCUx7@b;o1*C7LuhKFb7DXDu*o^gG9^<oWSzwL3y6R-k)Bsw|KPv)voig=sGsz%J$C>7ON#M{hfC8Z`h}#wI@%f z?#tWryX61!Inw7Ui=GDl`g@V7Uo-cMe|CLf?%(E~t^40^zjXTC@A;ofethL)KnD9h Ov#i+YGL=DtfdK#>VopN< literal 0 HcmV?d00001 diff --git a/frontend-plugin-core/src/test/resources/good.tgz b/frontend-plugin-core/src/test/resources/good.tgz new file mode 100644 index 0000000000000000000000000000000000000000..6d6153afad6ca7144ce08f38b1459f7774e7b1f9 GIT binary patch literal 146 zcmb2|=HNK!{W6w;IXyo=MXw~Wh~e!;L#}2Af!2pBZI8}7aAuwLJHaA8M*(i-8KOmh zaui&1l*ELX{)eB@PU~VfK2uyTd1}RH<|6jiD>DR04y%|sdUo}(5)D1oi8Vn2ovJ*TO literal 0 HcmV?d00001 From 605672bc470fe60e3e15104a7c8989a6486090c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armin=20H=C3=A4berling?= Date: Thu, 5 Dec 2019 21:09:00 +0100 Subject: [PATCH 2/3] Fix a regression in ArchiveExtractor that causes it to fail when extracting to a path containig a symlink This fixes a regression that was originaly fixed in commit 6175368 but reintroduced in commit 5a5eb07. --- .../maven/plugins/frontend/lib/ArchiveExtractor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArchiveExtractor.java b/frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArchiveExtractor.java index 921e498ec..cd0ee2dfb 100644 --- a/frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArchiveExtractor.java +++ b/frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ArchiveExtractor.java @@ -104,15 +104,15 @@ public void extract(String archive, String destinationDirectory) throws ArchiveE tarIn = new TarArchiveInputStream(new GzipCompressorInputStream(fis)); TarArchiveEntry tarEntry = tarIn.getNextTarEntry(); + String canonicalDestinationDirectory = new File(destinationDirectory).getCanonicalPath(); while (tarEntry != null) { // Create a file for this tarEntry final File destPath = new File(destinationDirectory + File.separator + tarEntry.getName()); prepDestination(destPath, tarEntry.isDirectory()); - - if (!startsWithPath(destPath.getCanonicalPath(), destinationDirectory)) { + if (!startsWithPath(destPath.getCanonicalPath(), canonicalDestinationDirectory)) { throw new IOException( - "Expanding " + tarEntry.getName() + " would create file outside of " + destinationDirectory + "Expanding " + tarEntry.getName() + " would create file outside of " + canonicalDestinationDirectory ); } From dea20862dc886499b7644714e1db141195647ace Mon Sep 17 00:00:00 2001 From: arminha Date: Fri, 6 Dec 2019 16:24:45 +0100 Subject: [PATCH 3/3] DefaultArchiveExtractorTest shouldn't fail when symlinks cannot be created Files.createSymbolicLink can also fail with an IOException when the user doesn't have the rights to create a symlink. For example on Windows 10 as normal user withouth admin rights. --- .../plugins/frontend/lib/DefaultArchiveExtractorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java b/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java index c50d8e777..5c7ff3aae 100644 --- a/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java +++ b/frontend-plugin-core/src/test/java/com/github/eirslett/maven/plugins/frontend/lib/DefaultArchiveExtractorTest.java @@ -57,10 +57,10 @@ public void extractBadTarFileSymlink() throws Exception { extractor.extract(BAD_TAR, link.toString()); } - private Path createSymlinkOrSkipTest(Path link, Path target) throws IOException { + private Path createSymlinkOrSkipTest(Path link, Path target) { try { return Files.createSymbolicLink(link, target); - } catch (UnsupportedOperationException e) { + } catch (UnsupportedOperationException | IOException e) { assumeTrue("symlinks not supported", false); return null; }