fix: cover the case of filename contain raise the directory traversal…

… vulnerability refs #1028
lingochamp · May 18, 2018 · e8dd772 · e8dd772
1 parent a37b329
commit e8dd772
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 4 deletions.
diff --git a/library/src/main/java/com/liulishuo/filedownloader/util/FileDownloadUtils.java b/library/src/main/java/com/liulishuo/filedownloader/util/FileDownloadUtils.java
@@ -648,10 +648,10 @@ public static String findFilename(FileDownloadConnection connection, String url)
 
         if (TextUtils.isEmpty(filename)) {
             filename = FileDownloadUtils.generateFileName(url);
-        } else if (filename.startsWith("../")) {
+        } else if (filename.contains("../")) {
             throw new FileDownloadSecurityException(FileDownloadUtils.formatString(
-                    "The filename [%s] from the response is not allowable, because it start"
-                            + " with '../', which can raise the directory traversal vulnerability",
+                    "The filename [%s] from the response is not allowable, because it is " +
+                            "contains '../', which can raise the directory traversal vulnerability",
                     filename));
         }
 

diff --git a/library/src/test/java/com/liulishuo/filedownloader/util/FileDownloadUtilsTest.java b/library/src/test/java/com/liulishuo/filedownloader/util/FileDownloadUtilsTest.java
@@ -19,7 +19,9 @@
 import com.liulishuo.filedownloader.connection.FileDownloadConnection;
 import com.liulishuo.filedownloader.exception.FileDownloadSecurityException;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 
@@ -68,13 +70,25 @@ public void parseContentLengthFromContentRange_withUnavailableContentRange() {
         assertThat(length).isEqualTo(-1);
     }
 
-    @Test(expected = FileDownloadSecurityException.class)
+    @Rule public ExpectedException thrown = ExpectedException.none();
+
+    @Test
     public void findFilename_securityIssue() throws FileDownloadSecurityException {
         final FileDownloadConnection connection = mock(FileDownloadConnection.class);
         when(connection.getResponseHeaderField("Content-Disposition"))
                 .thenReturn("attachment; filename=\"../abc\"");
 
+        thrown.expect(FileDownloadSecurityException.class);
         FileDownloadUtils.findFilename(connection, "url");
+
+        thrown.expect(FileDownloadSecurityException.class);
+        when(connection.getResponseHeaderField("Content-Disposition"))
+                .thenReturn("attachment; filename=\"a/b/../abc\"");
+        FileDownloadUtils.findFilename(connection, "url");
+
+        when(connection.getResponseHeaderField("Content-Disposition"))
+                .thenReturn("attachment; filename=\"/abc/adb\"");
+        assertThat(FileDownloadUtils.findFilename(connection, "url")).isEqualTo("/abc/adb");
     }
 
 }