Add method verifying one path is inside another, with tests (#4860)

Author: mikehardy

AnkiDroid/src/main/java/com/ichi2/libanki/Utils.java

@@ -26,6 +26,7 @@
 import android.content.pm.ResolveInfo;
 import android.content.res.Resources;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import android.text.Html;
 
 import com.ichi2.anki.AnkiFont;
@@ -631,6 +632,11 @@ public static void unzipFiles(ZipFile zipFile, String targetDirectory, String[]
                         name = zipEntryToFilenameMap.get(name);
                     }
                     File destFile = new File(dir, name);
+                    if (!isInside(destFile, dir)) {
+                        Timber.e("Refusing to decompress invalid path: " + destFile.getCanonicalPath());
+                        throw new IOException("File is outside extraction target directory.");
+                    }
+
                     if (!ze.isDirectory()) {
                         Timber.i("uncompress %s", name);
                         zis = new BufferedInputStream(zipFile.getInputStream(ze));
@@ -655,6 +661,18 @@ public static void unzipFiles(ZipFile zipFile, String targetDirectory, String[]
         }
     }
 
+    /**
+     * Checks to see if a given file path resides inside a given directory.
+     * Useful for protection against path traversal attacks prior to creating the file
+     * @param file the file with an uncertain filesystem location
+     * @param dir the directory that should contain the file
+     * @return true if the file path is inside the directory
+     * @exception IOException if there are security or filesystem issues determining the paths
+     */
+    public static boolean isInside(@NonNull File file, @NonNull File dir) throws IOException {
+        return file.getCanonicalPath().startsWith(dir.getCanonicalPath());
+    }
+
     /**
      * Compress data.
      * @param bytesToCompress is the byte array to compress.

AnkiDroid/src/test/java/com/ichi2/libanki/UtilsTest.java

@@ -0,0 +1,65 @@
+package com.ichi2.libanki;
+
+import junit.framework.Assert;
+
+//import net.lachlanmckee.timberjunit.TimberTestRule;
+
+//import org.junit.Rule;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.util.Enumeration;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+public class UtilsTest {
+
+    // This depends on a Timber upgrade that should be pursued separately
+    //@Rule
+    //public TimberTestRule logAllAlwaysRule = TimberTestRule.logAllAlways();
+
+    @Test
+    public void testZipWithPathTraversal() {
+
+        ClassLoader classLoader = getClass().getClassLoader();
+        URL resource = classLoader.getResource("path-traversal.zip");
+        File file = new File(resource.getPath());
+        try {
+            ZipFile zipFile = new ZipFile(file);
+            Enumeration zipEntries = zipFile.entries();
+            while (zipEntries.hasMoreElements()) {
+                ZipEntry ze2 = (ZipEntry) zipEntries.nextElement();
+                Utils.unzipFiles(zipFile, "/tmp", new String[]{ze2.getName()}, null);
+            }
+            Assert.fail("Expected an IOException");
+        }
+        catch (IOException e) {
+            Assert.assertEquals(e.getMessage(), "File is outside extraction target directory.");
+        }
+    }
+
+    @Test
+    public void testInvalidPaths() {
+        try {
+            File tmpDir = new File("/tmp");
+            Assert.assertFalse(Utils.isInside(new File(tmpDir, "../foo"), tmpDir));
+            Assert.assertFalse(Utils.isInside(new File(tmpDir, "/tmp/one/../../../foo"), tmpDir));
+        } catch (IOException ioe) {
+            Assert.fail("Unexpected exception: " + ioe);
+        }
+    }
+
+    @Test
+    public void testValidPaths() {
+        try {
+            File tmpDir = new File("/tmp");
+            Assert.assertTrue(Utils.isInside(new File(tmpDir, "test/file/path/no/parent"), tmpDir));
+            Assert.assertTrue(Utils.isInside(new File(tmpDir, "/tmp/absolute/path"), tmpDir));
+            Assert.assertTrue(Utils.isInside(new File(tmpDir, "test/file/../../"), tmpDir));
+        } catch (IOException ioe) {
+            Assert.fail("Unexpected exception: " + ioe);
+        }
+    }
+}