Path transversal (#32)

* Test failure in testReadFromValidCSVContentString.java on Windows

* Prevent path traversal exploit

* Fixed failed tests after fix for path traversal

src/main/java/io/frictionlessdata/tableschema/Table.java

@@ -19,17 +19,17 @@ public class Table{
     private DataSource dataSource = null;
     private Schema schema = null;
 
-    public Table(File dataSource) throws Exception{
-        this.dataSource = new CsvDataSource(dataSource);
+    public Table(File dataSource, File workDir) throws Exception{
+        this.dataSource = new CsvDataSource(dataSource, workDir);
     }
 
-    public Table(File dataSource, JSONObject schema) throws Exception{
-        this.dataSource = new CsvDataSource(dataSource);
+    public Table(File dataSource, File workDir, JSONObject schema) throws Exception{
+        this.dataSource = new CsvDataSource(dataSource, workDir);
         this.schema = new Schema(schema);
     }
 
-    public Table(File dataSource, Schema schema) throws Exception{
-        this.dataSource = new CsvDataSource(dataSource);
+    public Table(File dataSource, File workDir, Schema schema) throws Exception{
+        this.dataSource = new CsvDataSource(dataSource, workDir);
         this.schema = schema;
     }
 
@@ -103,6 +103,10 @@ public Iterator iterator(boolean keyed, boolean extended, boolean cast, boolean
     public String[] getHeaders() throws Exception{
         return this.dataSource.getHeaders();
     }
+
+    public void save(File outputFile) throws Exception{
+        this.dataSource.write(outputFile);
+    }
 
     public void save(String outputFilePath) throws Exception{
        this.dataSource.write(outputFilePath);

src/main/java/io/frictionlessdata/tableschema/datasources/CsvDataSource.java

@@ -1,38 +1,33 @@
 package io.frictionlessdata.tableschema.datasources;
 
 import com.google.common.collect.Iterators;
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileReader;
-import java.io.FileWriter;
-import java.io.Reader;
-import java.io.StringReader;
-import java.io.Writer;
+import org.apache.commons.csv.*;
+import org.json.CDL;
+import org.json.JSONArray;
+
+import java.io.*;
 import java.net.URL;
 import java.nio.charset.Charset;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import org.apache.commons.csv.CSVFormat;
-import org.apache.commons.csv.CSVParser;
-import org.apache.commons.csv.CSVPrinter;
-import org.apache.commons.csv.CSVRecord;
-import org.json.CDL;
-import org.json.JSONArray;
 
 /**
  *
  */
 public class CsvDataSource extends AbstractDataSource {
     private Object dataSource = null;
+    private File workDir;
 
     public CsvDataSource(URL dataSource){
         this.dataSource = dataSource;
     }
 
-    public CsvDataSource(File dataSource){
+    public CsvDataSource(File dataSource, File workDir){
         this.dataSource = dataSource;
+        this.workDir = workDir;
     }
 
     public CsvDataSource(String dataSource){
@@ -86,25 +81,30 @@ public List<String[]> data() throws Exception{
     }
 
     @Override
-    public void write(String outputFilePath) throws Exception{            
-       try(Writer out = new BufferedWriter(new FileWriter(outputFilePath));
-               CSVPrinter csvPrinter = new CSVPrinter(out, CSVFormat.RFC4180)) {
-            
+    public void write(File outputFile) throws Exception{
+        try(Writer out = new BufferedWriter(new FileWriter(outputFile));
+            CSVPrinter csvPrinter = new CSVPrinter(out, CSVFormat.RFC4180)) {
+
             if(this.getHeaders() != null){
                 csvPrinter.printRecord(this.getHeaders());
             }
-            
+
             Iterator<CSVRecord> recordIter = this.getCSVParser().iterator();
             while(recordIter.hasNext()){
                 CSVRecord record = recordIter.next();
                 csvPrinter.printRecord(record);
             }
-            
+
             csvPrinter.flush();
-                
-       }catch(Exception e){
+
+        }catch(Exception e){
             throw e;
-       }
+        }
+    }
+
+    @Override
+    public void write(String outputFilePath) throws Exception{
+        write(new File(outputFilePath));
     }
 
     @Override
@@ -148,13 +148,16 @@ private CSVParser getCSVParser() throws Exception{
         }else if(this.dataSource instanceof File){
             // The path value can either be a relative path or a full path.
             // If it's a relative path then build the full path by using the working directory.
-            File f = (File)this.dataSource;
-            if(!f.exists()) { 
-                f = new File(System.getProperty("user.dir") + "/" + f.getAbsolutePath());
-            }
+            // Caution: here, we cannot simply use provided paths, we have to check
+            // they are neither absolute path or relative parent paths (../)
+            // see:
+            //    - https://github.com/frictionlessdata/tableschema-java/issues/29
+            //    - https://frictionlessdata.io/specs/data-resource/#url-or-path
+            Path inPath = ((File)this.dataSource).toPath();
+            Path resolvedPath = DataSource.toSecure(inPath, workDir.toPath());
 
             // Read the file.
-            Reader fr = new FileReader(f);
+            Reader fr = new FileReader(resolvedPath.toFile());
 
             // Get the parser.
             return CSVFormat.RFC4180.withHeader().parse(fr);

src/main/java/io/frictionlessdata/tableschema/datasources/DataSource.java

@@ -3,6 +3,9 @@
  */
 package io.frictionlessdata.tableschema.datasources;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
 import java.util.Iterator;
 import java.util.List;
 
@@ -13,5 +16,28 @@ public interface DataSource {
     public Iterator<String[]> iterator() throws Exception;
     public String[] getHeaders() throws Exception;
     public List<String[]> data() throws Exception;
-    public void write(String outputFilePath) throws Exception;
+    public void write(String outputFile) throws Exception;
+    public void write(File outputFile) throws Exception;
+
+    public static Path toSecure(Path testPath, Path referencePath) throws IOException {
+        if (!referencePath.isAbsolute()) {
+            throw new IllegalArgumentException("Reference path must be absolute");
+        }
+        if (testPath.isAbsolute()){
+            throw new IllegalArgumentException("Input path must be relative");
+        }
+        if (testPath.toFile().isDirectory()){
+            throw new IllegalArgumentException("Input path cannot be a directory");
+        }
+        //Path canonicalPath = testPath.toRealPath(null);
+        final Path resolvedPath = referencePath.resolve(testPath).normalize();
+        if (!resolvedPath.toFile().isFile()){
+            throw new IllegalArgumentException("Input must be a file");
+        }
+        if (!resolvedPath.startsWith(referencePath)) {
+            throw new IllegalArgumentException("Input path escapes the base path");
+        }
+
+        return resolvedPath;
+    }
 }

src/test/java/io/frictionlessdata/tableschema/DataSourceTest.java

@@ -0,0 +1,60 @@
+package io.frictionlessdata.tableschema;
+
+import com.sun.javafx.PlatformUtil;
+import io.frictionlessdata.tableschema.datasources.DataSource;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import java.io.File;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+public class DataSourceTest {
+    @Rule
+    public final ExpectedException exception = ExpectedException.none();
+
+    @Test
+    public void testUnsafePath1() throws Exception {
+       URL u = DataSourceTest.class.getResource("/fixtures/dates_data.csv");
+       Path path = Paths.get(u.toURI());
+       Path testPath = path.getParent();
+       String maliciousPathName = "/etc/passwd";
+       if (PlatformUtil.isWindows()){
+           maliciousPathName = "C:/Windows/system.ini";
+       }
+       Path maliciousPath = new File(maliciousPathName).toPath();
+       exception.expect(IllegalArgumentException.class);
+       DataSource.toSecure(maliciousPath, testPath);
+    }
+
+    @Test
+    public void testUnsafePath2() throws Exception {
+        URL u = DataSourceTest.class.getResource("/fixtures/dates_data.csv");
+        Path path = Paths.get(u.toURI());
+        Path testPath = path.getParent();
+        String maliciousPathName = "/etc/";
+        if (PlatformUtil.isWindows()){
+            maliciousPathName = "C:/Windows/";
+        }
+        Path maliciousPath = new File(maliciousPathName).toPath();
+        exception.expect(IllegalArgumentException.class);
+        DataSource.toSecure(maliciousPath, testPath);
+    }
+
+    @Test
+    public void testSafePath() throws Exception {
+        URL u = DataSourceTest.class.getResource("/fixtures/dates_data.csv");
+        Path path = Paths.get(u.toURI());
+        Path testPath = path.getParent().getParent();
+        String maliciousPathName = "fixtures/dates_data.csv";
+        if (PlatformUtil.isWindows()){
+            maliciousPathName = "fixtures/dates_data.csv";
+        }
+        Path maliciousPath = new File(maliciousPathName).toPath();
+        DataSource.toSecure(maliciousPath, testPath);
+    }
+}
+