From f807c40d8cee00735b479a0e9186275e881e87c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armin=20H=C3=A4berling?= Date: Fri, 17 May 2019 09:40:18 +0200 Subject: [PATCH 1/2] Fix indentation in ArchiveExtractor --- .../frontend/lib/ArchiveExtractor.java | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 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 64f261f3c..901af7296 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 @@ -81,53 +81,53 @@ public void extract(String archive, String destinationDirectory) throws ArchiveE final File destPath = new File(destinationDirectory + File.separator + entry.getName()); prepDestination(destPath, entry.isDirectory()); if(!entry.isDirectory()){ - InputStream in = null; - OutputStream out = null; - try { - in = zipFile.getInputStream(entry); - out = new FileOutputStream(destPath); - IOUtils.copy(in, out); - } finally { - IOUtils.closeQuietly(in); - IOUtils.closeQuietly(out); - } + InputStream in = null; + OutputStream out = null; + try { + in = zipFile.getInputStream(entry); + out = new FileOutputStream(destPath); + IOUtils.copy(in, out); + } finally { + IOUtils.closeQuietly(in); + IOUtils.closeQuietly(out); + } } } } finally { zipFile.close(); } } else { - // TarArchiveInputStream can be constructed with a normal FileInputStream if - // we ever need to extract regular '.tar' files. + // TarArchiveInputStream can be constructed with a normal FileInputStream if + // we ever need to extract regular '.tar' files. TarArchiveInputStream tarIn = null; try { - tarIn = new TarArchiveInputStream(new GzipCompressorInputStream(fis)); + tarIn = new TarArchiveInputStream(new GzipCompressorInputStream(fis)); - TarArchiveEntry tarEntry = tarIn.getNextTarEntry(); - while (tarEntry != null) { - // Create a file for this tarEntry - final File destPath = new File(destinationDirectory + File.separator + tarEntry.getName()); - prepDestination(destPath, tarEntry.isDirectory()); - if (!destPath.getCanonicalPath().startsWith(destinationDirectory)) { - throw new IOException( - "Expanding " + tarEntry.getName() + " would create file outside of " + destinationDirectory - ); - } - if (!tarEntry.isDirectory()) { - destPath.createNewFile(); - boolean isExecutable = (tarEntry.getMode() & 0100) > 0; - destPath.setExecutable(isExecutable); + TarArchiveEntry tarEntry = tarIn.getNextTarEntry(); + while (tarEntry != null) { + // Create a file for this tarEntry + final File destPath = new File(destinationDirectory + File.separator + tarEntry.getName()); + prepDestination(destPath, tarEntry.isDirectory()); + if (!destPath.getCanonicalPath().startsWith(destinationDirectory)) { + throw new IOException( + "Expanding " + tarEntry.getName() + " would create file outside of " + destinationDirectory + ); + } + if (!tarEntry.isDirectory()) { + destPath.createNewFile(); + boolean isExecutable = (tarEntry.getMode() & 0100) > 0; + destPath.setExecutable(isExecutable); - OutputStream out = null; - try { - out = new FileOutputStream(destPath); - IOUtils.copy(tarIn, out); - } finally { - IOUtils.closeQuietly(out); - } - } - tarEntry = tarIn.getNextTarEntry(); - } + OutputStream out = null; + try { + out = new FileOutputStream(destPath); + IOUtils.copy(tarIn, out); + } finally { + IOUtils.closeQuietly(out); + } + } + tarEntry = tarIn.getNextTarEntry(); + } } finally { IOUtils.closeQuietly(tarIn); } From 6175368cca93620bad45260152391b360d0751db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armin=20H=C3=A4berling?= Date: Fri, 17 May 2019 09:44:36 +0200 Subject: [PATCH 2/2] Compare paths when extracting an archive with the canonical destination directory Commit 93d77ffc023effbcb36813648b578a0541709d76 introduced a check that files extracted from a tar archive will not be written outside of the destination directory. Unfortunately it also introduced a regression prventing the extraction of any tar archive when the path to the destination directory contains a symbolic link. For example on a jenkins agent where /var/lib/jenkins is a symbolic link to /data/jenkins and a job tries to extact nodejs to /var/lib/jenkins/workspace/example-project/target/node/tmp. The old code would then check if canonical path to a tar entry like /data/jenkins/workspace/example-project/target/node/tmp/XXX starts with /var/lib/jenkins/workspace/example-project/target/node/tmp which always fails. This commit compares the canonical extraction paths of the tar entries with the canonical path of the destination directory, which fixes the regression and still checks that no file is extracted outside of the destination directory. --- .../maven/plugins/frontend/lib/ArchiveExtractor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 901af7296..e0bbe8ae7 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,13 +104,14 @@ 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 (!destPath.getCanonicalPath().startsWith(destinationDirectory)) { + if (!destPath.getCanonicalPath().startsWith(canonicalDestinationDirectory)) { throw new IOException( - "Expanding " + tarEntry.getName() + " would create file outside of " + destinationDirectory + "Expanding " + tarEntry.getName() + " would create file outside of " + canonicalDestinationDirectory ); } if (!tarEntry.isDirectory()) {