diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Utils.java b/AnkiDroid/src/main/java/com/ichi2/libanki/Utils.java index fcb850e6553a..2a21f4edc8f1 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Utils.java +++ b/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. diff --git a/AnkiDroid/src/test/java/com/ichi2/libanki/UtilsTest.java b/AnkiDroid/src/test/java/com/ichi2/libanki/UtilsTest.java new file mode 100644 index 000000000000..7ee4a919bd22 --- /dev/null +++ b/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); + } + } +} diff --git a/AnkiDroid/src/test/resources/path-traversal.zip b/AnkiDroid/src/test/resources/path-traversal.zip new file mode 100644 index 000000000000..ad9bd111c94a Binary files /dev/null and b/AnkiDroid/src/test/resources/path-traversal.zip differ