Sanitize download attribute file names in DownloadingAttachmentHandler

src/main/java/org/htmlunit/attachment/Attachment.java

@@ -29,6 +29,7 @@
  * @author Daniel Gredler
  * @author Ronald Brill
  * @author Lai Quang Duong
+ * @author Vidya m
  */
 public class Attachment {
 
@@ -145,7 +146,7 @@ public static String getSuggestedFilename(final String contentDispositionHeader)
      * @param name the name to sanitize
      * @return the sanitized name or {@code null} if nothing is left
      */
-    private static String sanitizeFilename(final String name) {
+    static String sanitizeFilename(final String name) {
         if (name == null) {
             return null;
         }

src/main/java/org/htmlunit/attachment/DownloadingAttachmentHandler.java

@@ -39,6 +39,7 @@
  *
  * @author Marek Andreansky
  * @author Ronald Brill
+ * @author Vidya m
  */
 public class DownloadingAttachmentHandler implements AttachmentHandler {
 
@@ -96,7 +97,9 @@
     }
 
     private Path determineDestionationFile(final Page page, final String attachmentFilename) {
-        String fileName = attachmentFilename;
+        // the name can come from the page (download attribute), sanitize it like
+        // a name from the Content-Disposition header to stay inside the download folder
+        String fileName = Attachment.sanitizeFilename(attachmentFilename);
 
         if (StringUtils.isBlank(fileName)) {
             final String file = page.getWebResponse().getWebRequest().getUrl().getFile();

src/test/java/org/htmlunit/attachment/DownloadingAttachmentHandlerTest.java

@@ -36,6 +36,7 @@
  *
  * @author Marek Andreansky
  * @author Ronald Brill
+ * @author Vidya m
  */
 public class DownloadingAttachmentHandlerTest extends SimpleWebTestCase {
 
@@ -207,4 +208,60 @@ public void basicFileNameFromHeader() throws Exception {
             FileUtils.deleteDirectory(downloadFolder);
         }
     }
+
+    /**
+     * The file name from the download attribute is sanitized like the one from the
+     * Content-Disposition header; a path can't be used to leave the download folder.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void fileNameFromDownloadAttributeIsSanitized() throws Exception {
+        final String content1 = DOCTYPE_HTML
+            + "<html><body>\n"
+            + "<a id='relative' href='" + URL_SECOND + "'>relative</a>\n"
+            + "<a id='absolute' href='" + URL_SECOND + "'>absolute</a>\n"
+            + "</body></html>";
+        final String content2 = "download file contents";
+
+        final File tmpFolder = new File(System.getProperty("java.io.tmpdir"));
+        final File testFolder = new File(tmpFolder, "downloading-attachment-test");
+        final File downloadFolder = new File(testFolder, "downloads");
+        final File absoluteFile = new File(testFolder, "absolute.txt");
+        FileUtils.deleteDirectory(testFolder);
+
+        try {
+            FileUtils.forceMkdir(downloadFolder);
+
+            final WebClient client = getWebClient();
+            client.setAttachmentHandler(new DownloadingAttachmentHandler(downloadFolder.toPath()));
+
+            final MockWebConnection conn = new MockWebConnection();
+            conn.setResponse(URL_FIRST, content1);
+            conn.setResponse(URL_SECOND, content2, MimeType.TEXT_PLAIN);
+            client.setWebConnection(conn);
+
+            final HtmlPage page = client.getPage(URL_FIRST);
+
+            final HtmlAnchor relative = page.getHtmlElementById("relative");
+            relative.setAttribute("download", "../evil.txt");
+            relative.click();
+
+            final HtmlAnchor absolute = page.getHtmlElementById("absolute");
+            absolute.setAttribute("download", absoluteFile.getAbsolutePath());
+            absolute.click();
+
+            assertFalse(absoluteFile.exists());
+
+            final Collection<File> files = FileUtils.listFiles(testFolder, null, true);
+            assertEquals(2, files.size());
+            for (final File file : files) {
+                assertEquals(downloadFolder, file.getParentFile());
+            }
+            assertTrue(files.removeIf(f -> f.getName().equals("_evil.txt")));
+        }
+        finally {
+            FileUtils.deleteDirectory(testFolder);
+        }
+    }
 }