fix: Directory Traversal Vulnerability

Closes #83

okdownload-filedownloader/src/main/java/com/liulishuo/filedownloader/CompatListenerAssist.java

@@ -24,11 +24,13 @@
 
 import com.liulishuo.filedownloader.exception.FileDownloadNetworkPolicyException;
 import com.liulishuo.filedownloader.exception.FileDownloadOutOfSpaceException;
+import com.liulishuo.filedownloader.exception.FileDownloadSecurityException;
 import com.liulishuo.filedownloader.retry.RetryAssist;
 import com.liulishuo.filedownloader.util.FileDownloadUtils;
 import com.liulishuo.okdownload.DownloadTask;
 import com.liulishuo.okdownload.core.Util;
 import com.liulishuo.okdownload.core.cause.EndCause;
+import com.liulishuo.okdownload.core.exception.DownloadSecurityException;
 import com.liulishuo.okdownload.core.exception.NetworkPolicyException;
 import com.liulishuo.okdownload.core.exception.PreAllocateException;
 
@@ -183,6 +185,8 @@ void handleError(
                     preAllocateException.getRequireSpace(),
                     downloadTaskAdapter.getProgressAssist().getSofarBytes(),
                     preAllocateException);
+        } else if (realCause instanceof DownloadSecurityException) {
+            throwable = new FileDownloadSecurityException(realCause.getMessage());
         } else {
             throwable = new Throwable(realCause);
         }

okdownload-filedownloader/src/test/java/com.liulishuo.filedownloader/CompatListenerAssistTest.java

@@ -20,10 +20,12 @@
 
 import com.liulishuo.filedownloader.exception.FileDownloadNetworkPolicyException;
 import com.liulishuo.filedownloader.exception.FileDownloadOutOfSpaceException;
+import com.liulishuo.filedownloader.exception.FileDownloadSecurityException;
 import com.liulishuo.filedownloader.progress.ProgressAssist;
 import com.liulishuo.filedownloader.retry.RetryAssist;
 import com.liulishuo.okdownload.DownloadTask;
 import com.liulishuo.okdownload.core.cause.EndCause;
+import com.liulishuo.okdownload.core.exception.DownloadSecurityException;
 import com.liulishuo.okdownload.core.exception.NetworkPolicyException;
 import com.liulishuo.okdownload.core.exception.PreAllocateException;
 
@@ -313,6 +315,28 @@ public void handleError_withPreAllocateException() {
                 .isExactlyInstanceOf(FileDownloadOutOfSpaceException.class);
     }
 
+    @Test
+    public void handleError_withDownloadSecurityException() {
+        final DownloadTaskAdapter mockTaskAdapter = mock(DownloadTaskAdapter.class);
+        final ProgressAssist mockProgressAssist = mock(ProgressAssist.class);
+        when(mockTaskAdapter.getProgressAssist()).thenReturn(mockProgressAssist);
+        when(mockTaskAdapter.getRetryAssist()).thenReturn(null);
+        when(mockProgressAssist.getSofarBytes()).thenReturn(1L);
+        final Exception mockException = mock(DownloadSecurityException.class);
+
+        compatListenerAssist.handleError(mockTaskAdapter, mockException);
+
+        verify(callback, never()).retry(
+                any(DownloadTaskAdapter.class), any(Throwable.class), anyInt(), anyLong());
+        final ArgumentCaptor<Throwable> throwableCaptor = ArgumentCaptor.forClass(Throwable.class);
+        final ArgumentCaptor<DownloadTaskAdapter> taskCaptor = ArgumentCaptor
+                .forClass(DownloadTaskAdapter.class);
+        verify(callback).error(taskCaptor.capture(), throwableCaptor.capture());
+        assertThat(taskCaptor.getValue()).isEqualTo(mockTaskAdapter);
+        assertThat(throwableCaptor.getValue())
+                .isExactlyInstanceOf(FileDownloadSecurityException.class);
+    }
+
     @Test
     public void handleError_withOtherException() {
         final DownloadTaskAdapter mockTaskAdapter = mock(DownloadTaskAdapter.class);

okdownload/src/main/java/com/liulishuo/okdownload/core/download/ConnectTrial.java

@@ -26,6 +26,7 @@
 import com.liulishuo.okdownload.core.Util;
 import com.liulishuo.okdownload.core.breakpoint.BreakpointInfo;
 import com.liulishuo.okdownload.core.connection.DownloadConnection;
+import com.liulishuo.okdownload.core.exception.DownloadSecurityException;
 
 import java.io.IOException;
 import java.net.HttpURLConnection;
@@ -176,7 +177,8 @@ private static boolean isAcceptRange(@NonNull DownloadConnection.Connected conne
         return "bytes".equals(acceptRanges);
     }
 
-    @Nullable private static String findFilename(DownloadConnection.Connected connected) {
+    @Nullable private static String findFilename(DownloadConnection.Connected connected)
+        throws IOException {
         return parseContentDisposition(connected.getResponseHeaderField(CONTENT_DISPOSITION));
     }
 
@@ -194,21 +196,31 @@ private static boolean isAcceptRange(@NonNull DownloadConnection.Connected conne
      * This header provides a filename for content that is going to be
      * downloaded to the file system. We only support the attachment type.
      */
-    @Nullable private static String parseContentDisposition(String contentDisposition) {
+    @Nullable private static String parseContentDisposition(String contentDisposition)
+            throws IOException {
         if (contentDisposition == null) {
             return null;
         }
 
         try {
+            String fileName = null;
             Matcher m = CONTENT_DISPOSITION_QUOTED_PATTERN.matcher(contentDisposition);
             if (m.find()) {
-                return m.group(1);
+                fileName = m.group(1);
+            } else  {
+                m = CONTENT_DISPOSITION_NON_QUOTED_PATTERN.matcher(contentDisposition);
+                if (m.find()) {
+                    fileName = m.group(1);
+                }
             }
 
-            m = CONTENT_DISPOSITION_NON_QUOTED_PATTERN.matcher(contentDisposition);
-            if (m.find()) {
-                return m.group(1);
+            if (fileName != null && fileName.contains("../")) {
+                throw new DownloadSecurityException("The filename [" + fileName + "] from"
+                    + " the response is not allowable, because it contains '../', which "
+                    + "can raise the directory traversal vulnerability");
             }
+
+            return fileName;
         } catch (IllegalStateException ex) {
             // This function is defined as returning null when it can't parse the header
         }

okdownload/src/main/java/com/liulishuo/okdownload/core/exception/DownloadSecurityException.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2018 LingoChamp Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.liulishuo.okdownload.core.exception;
+
+import java.io.IOException;
+
+public class DownloadSecurityException extends IOException {
+
+    public DownloadSecurityException(String message) {
+        super(message);
+    }
+}
+

okdownload/src/test/java/com/liulishuo/okdownload/core/download/ConnectTrialTest.java

@@ -22,9 +22,12 @@
 import com.liulishuo.okdownload.core.breakpoint.BreakpointInfo;
 import com.liulishuo.okdownload.core.connection.DownloadConnection;
 import com.liulishuo.okdownload.core.dispatcher.CallbackDispatcher;
+import com.liulishuo.okdownload.core.exception.DownloadSecurityException;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.Mock;
 
 import java.io.IOException;
@@ -67,6 +70,7 @@ public class ConnectTrialTest {
     @Mock private BreakpointInfo info;
     @Mock private DownloadConnection connection;
     @Mock private DownloadConnection.Connected connected;
+    @Rule public ExpectedException thrown= ExpectedException.none();
 
     private final String etag = "etag";
     private final String url = "https://jacksgong.com";
@@ -212,6 +216,22 @@ public void getResponseFilename() throws IOException {
                 .thenReturn("attachment; filename=genome.jpeg\nabc");
         connectTrial.executeTrial();
         assertThat(connectTrial.getResponseFilename()).isEqualTo("genome.jpeg");
+
+        when(connected.getResponseHeaderField(CONTENT_DISPOSITION))
+                .thenReturn("attachment; filename=../data/data/genome.png");
+        thrown.expect(DownloadSecurityException.class);
+        thrown.expectMessage("The filename [../data/data/genome.png] from the response is not "
+                + "allowable, because it contains '../', which can raise the directory traversal "
+                + "vulnerability");
+        connectTrial.executeTrial();
+
+        when(connected.getResponseHeaderField(CONTENT_DISPOSITION))
+                .thenReturn("attachment; filename=a/b/../abc");
+        thrown.expect(DownloadSecurityException.class);
+        thrown.expectMessage("The filename [a/b/../abc] from the response is not "
+                + "allowable, because it contains '../', which can raise the directory traversal "
+                + "vulnerability");
+        connectTrial.executeTrial();
     }
 
     @Test