Merge branch 'master' into 2.0.x

* master: #1325 - Avoid datasets being extracted outside their target directory #1325 - Avoid datasets being extracted outside their target directory #1325 - Avoid datasets being extracted outside their target directory #1338 - Factor CAS <-> brat conversion code into Pojos #1338 - Factor CAS <-> brat conversion code into Pojos #1325 - Avoid datasets being extracted outside their target directory #1325 - Avoid datasets being extracted outside their target directory
dkpro · Apr 16, 2019 · 7af21c7 · 7af21c7
2 parents b52ef60 + 9c4c5d5
commit 7af21c7
Show file tree

Hide file tree

Showing 5 changed files with 681 additions and 468 deletions.
diff --git a/...-datasets-asl/src/main/java/de/tudarmstadt/ukp/dkpro/core/api/datasets/DatasetLoader.java b/...-datasets-asl/src/main/java/de/tudarmstadt/ukp/dkpro/core/api/datasets/DatasetLoader.java
@@ -24,6 +24,7 @@
 import java.io.InputStream;
 import java.net.URL;
 import java.net.URLConnection;
+import java.nio.file.Path;
 import java.security.DigestInputStream;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
@@ -250,12 +251,20 @@ private void extract(File aArchive, ArchiveInputStream aArchiveStream, File aTar
                 throw new IllegalStateException("Filename must not contain line break");
             }
 
-            File out = new File(aTarget, name);
+            Path base = aTarget.toPath().toAbsolutePath();
+            Path out = base.resolve(name).toAbsolutePath();
+
+            if (!out.startsWith(base)) {
+                // Ignore attempts to write outside the base
+                continue;
+            }
+
             if (entry.isDirectory()) {
-                FileUtils.forceMkdir(out);
+                FileUtils.forceMkdir(out.toFile());
             }
             else {
-                FileUtils.copyInputStreamToFile(new CloseShieldInputStream(aArchiveStream), out);
+                FileUtils.copyInputStreamToFile(new CloseShieldInputStream(aArchiveStream),
+                        out.toFile());
             }
         }
     }

diff --git a/...sl/src/main/java/de/tudarmstadt/ukp/dkpro/core/api/datasets/internal/actions/Explode.java b/...sl/src/main/java/de/tudarmstadt/ukp/dkpro/core/api/datasets/internal/actions/Explode.java
@@ -39,6 +39,8 @@
 import org.apache.commons.compress.compressors.CompressorStreamFactory;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import com.github.junrar.Archive;
 import com.github.junrar.exception.RarException;
@@ -54,6 +56,8 @@
 public class Explode
     extends Action_ImplBase
 {
+    private final Log LOG = LogFactory.getLog(getClass());
+
     @Override
     public void apply(ActionDescription aAction, DatasetDescription aDataset,
             ArtifactDescription aPack, Path aCachedFile)
@@ -99,19 +103,22 @@ public void apply(ActionDescription aAction, DatasetDescription aDataset,
         }
     }
 
-    private void extract7z(ActionDescription aAction, Path aCachedFile, Path aTarget)
+    private void extract7z(ActionDescription aAction, Path aArchive, Path aTarget)
         throws IOException, RarException
     {
         // We always extract archives into a subfolder. Figure out the name of the folder.
-        String base = getBase(aCachedFile.getFileName().toString());
+        Path base = aTarget.resolve(getPathWithoutFileExtension(aArchive)).toAbsolutePath();
 
         Map<String, Object> cfg = aAction.getConfiguration();
         int strip = cfg.containsKey("strip") ? (int) cfg.get("strip") : 0;
 
         AntFileFilter filter = new AntFileFilter(coerceToList(cfg.get("includes")),
                 coerceToList(cfg.get("excludes")));
 
-        try (SevenZFile archive = new SevenZFile(aCachedFile.toFile())) {
+        LOG.info("Extracting files of [" + aArchive.getFileName() + "] to [" + aTarget.resolve(base)
+                + "]");
+
+        try (SevenZFile archive = new SevenZFile(aArchive.toFile())) {
             SevenZArchiveEntry entry = archive.getNextEntry();
             while (entry != null) {
                 String name = stripLeadingFolders(entry.getName(), strip);
@@ -122,7 +129,13 @@ private void extract7z(ActionDescription aAction, Path aCachedFile, Path aTarget
                 }
 
                 if (filter.accept(name)) {
-                    Path out = aTarget.resolve(base).resolve(name);
+                    Path out = base.resolve(name).toAbsolutePath();
+                    if (!out.startsWith(base)) {
+                        throw new IOException(
+                                "Archive tries to generate file outside target folder: [" + name
+                                        + "]");
+                    }
+
                     if (entry.isDirectory()) {
                         Files.createDirectories(out);
                     }
@@ -140,19 +153,22 @@ private void extract7z(ActionDescription aAction, Path aCachedFile, Path aTarget
         }
     }
 
-    private void extractRar(ActionDescription aAction, Path aCachedFile, Path aTarget)
+    private void extractRar(ActionDescription aAction, Path aArchive, Path aTarget)
         throws IOException, RarException
     {
         // We always extract archives into a subfolder. Figure out the name of the folder.
-        String base = getBase(aCachedFile.getFileName().toString());
+        Path base = aTarget.resolve(getPathWithoutFileExtension(aArchive)).toAbsolutePath();
 
         Map<String, Object> cfg = aAction.getConfiguration();
         int strip = cfg.containsKey("strip") ? (int) cfg.get("strip") : 0;
 
         AntFileFilter filter = new AntFileFilter(coerceToList(cfg.get("includes")),
                 coerceToList(cfg.get("excludes")));
 
-        try (Archive archive = new Archive(new FileVolumeManager(aCachedFile.toFile()))) {
+        LOG.info("Extracting files of [" + aArchive.getFileName() + "] to [" + aTarget.resolve(base)
+                + "]");
+
+        try (Archive archive = new Archive(new FileVolumeManager(aArchive.toFile()))) {
             FileHeader fh = archive.nextFileHeader();
             while (fh != null) {
                 String name = stripLeadingFolders(fh.getFileNameString(), strip);
@@ -163,7 +179,13 @@ private void extractRar(ActionDescription aAction, Path aCachedFile, Path aTarge
                 }
 
                 if (filter.accept(name)) {
-                    Path out = aTarget.resolve(base).resolve(name);
+                    Path out = base.resolve(name).toAbsolutePath();
+                    if (!out.startsWith(base)) {
+                        throw new IOException(
+                                "Archive tries to generate file outside target folder: [" + name
+                                        + "]");
+                    }
+
                     if (fh.isDirectory()) {
                         Files.createDirectories(out);
                     }
@@ -185,14 +207,17 @@ private void extract(ActionDescription aAction, Path aArchive, ArchiveInputStrea
         throws IOException
     {
         // We always extract archives into a subfolder. Figure out the name of the folder.
-        String base = getBase(aArchive.getFileName().toString());
+        Path base = aTarget.resolve(getPathWithoutFileExtension(aArchive)).toAbsolutePath();
 
         Map<String, Object> cfg = aAction.getConfiguration();
         int strip = cfg.containsKey("strip") ? (int) cfg.get("strip") : 0;
 
         AntFileFilter filter = new AntFileFilter(coerceToList(cfg.get("includes")),
                 coerceToList(cfg.get("excludes")));
 
+        LOG.info("Extracting files of [" + aArchive.getFileName() + "] to [" + aTarget.resolve(base)
+                + "]");
+
         ArchiveEntry entry = null;
         while ((entry = aAStream.getNextEntry()) != null) {
             String name = stripLeadingFolders(entry.getName(), strip);
@@ -203,7 +228,12 @@ private void extract(ActionDescription aAction, Path aArchive, ArchiveInputStrea
             }
 
             if (filter.accept(name)) {
-                Path out = aTarget.resolve(base).resolve(name);
+                Path out = base.resolve(name).toAbsolutePath();
+                if (!out.startsWith(base)) {
+                    throw new IOException(
+                            "Archive tries to generate file outside target folder: [" + name + "]");
+                }
+
                 if (entry.isDirectory()) {
                     Files.createDirectories(out);
                 }
@@ -217,6 +247,10 @@ private void extract(ActionDescription aAction, Path aArchive, ArchiveInputStrea
 
     private String stripLeadingFolders(String aName, int aLevels)
     {
+        if (aName == null) {
+            return null;
+        }
+
         if (aLevels > 0) {
             Path p = Paths.get(aName);
             if (p.getNameCount() <= aLevels) {
@@ -233,10 +267,16 @@ private String stripLeadingFolders(String aName, int aLevels)
         }
     }
 
-    public static String getBase(String aFilename)
+    /**
+     * The the name of the archive without any extensions (e.g. in the case of multiple extensions
+     * such as .tar.gz).
+     */
+    public static String getPathWithoutFileExtension(Path aFilename)
     {
+
+
         // We always extract archives into a subfolder. Figure out the name of the folder.
-        String base = aFilename;
+        String base = aFilename.getFileName().toString();
         while (base.contains(".")) {
             base = FilenameUtils.removeExtension(base);
         }

diff --git a/dkpro-core-io-brat-asl/pom.xml b/dkpro-core-io-brat-asl/pom.xml
@@ -46,6 +46,14 @@
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
     </dependency>
+    <dependency>
+      <groupId>commons-logging</groupId>
+      <artifactId>commons-logging-api</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>de.tudarmstadt.ukp.dkpro.core</groupId>
+      <artifactId>de.tudarmstadt.ukp.dkpro.core.api.segmentation-asl</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-core</artifactId>