Remove unsafe Conent-Disposition inferring

src/command/cmd_funcs.c

@@ -9164,46 +9164,23 @@ cmd_url_open(ProfWin* window, const char* const command, gchar** args)
 }
 
 
-void _url_save_fallback_method(ProfWin *window, const char *url, const char *directory, const char *filename) {
+void _url_save_fallback_method(ProfWin *window, const char *url, const char *filename) {
+    FILE *fh = fopen(filename, "wb");
+    if (!fh) {
+        cons_show_error("Cannot open file '%s' for writing.", filename);
+        return;
+    }
+
     HTTPDownload *download = malloc(sizeof(HTTPDownload));
     download->window = window;
     download->url = strdup(url);
-
+    download->filehandle = fh;
-    if (filename) {
-        download->filename = strdup(filename);
-    } else {
-        download->filename = NULL;
-    }
-
-    if (directory) {
-        download->directory = strdup(directory);
-    } else {
-        download->directory = NULL;
-    }
 
     pthread_create(&(download->worker), NULL, &http_file_get, download);
     http_download_add_download(download);
 }
 
-void _url_save_external_method(const char *scheme_cmd, const char *url, const char *directory, char *filename) {
+void _url_save_external_method(const char *scheme_cmd, const char *url, const char *filename) {
-    if (!filename) {
-        filename = http_filename_from_url(url);
-    }
-
-    // Explicitly use "." as directory if no directory has been passed.
-    char *fp = NULL;
-    if (directory == NULL) {
-        fp = g_build_filename(".", filename, NULL);
-    } else {
-        fp = g_build_filename(directory, filename, NULL);
-    }
-
-    if (!g_file_test(directory, G_FILE_TEST_EXISTS) ||
-        !g_file_test(directory, G_FILE_TEST_IS_DIR)) {
-        cons_show_error("Directory '%s' does not exist or is not a directory.", directory);
-        return;
-    }
-
     gchar **argv = g_strsplit(scheme_cmd, " ", 0);
 
     guint num_args = 0;
@@ -9213,7 +9190,7 @@ void _url_save_external_method(const char *scheme_cmd, const char *url, const ch
             argv[num_args] = g_strdup(url);
         } else if (0 == g_strcmp0(argv[num_args], "%p")) {
             g_free(argv[num_args]);
-            argv[num_args] = fp;
+            argv[num_args] = strdup(filename);
         }
         num_args++;
     }
@@ -9221,10 +9198,29 @@ void _url_save_external_method(const char *scheme_cmd, const char *url, const ch
     if (!call_external(argv, NULL, NULL)) {
         cons_show_error("Unable to save url: check the logs for more information.");
     } else {
-        cons_show("URL '%s' has been saved to '%s'.", url, fp);
+        cons_show("URL '%s' has been saved to '%s'.", url, filename);
     }
 }
 
+char *_make_unique_filename(const char *filename) {
+    char *unique = strdup(filename);
+
+    unsigned int i = 0;
+    while(g_file_test(unique, G_FILE_TEST_EXISTS)) {
+        free(unique);
+
+        if (i > 1000) { // Give up after 1000 attempts.
+            return NULL;
+        }
+
+        if (asprintf(&unique, "%s.%u", filename, i) < 0) {
+            return NULL;
+        }
+    }
+
+    return unique;
+}
+
 gboolean
 cmd_url_save(ProfWin *window, const char *const command, gchar **args)
 {
@@ -9251,23 +9247,50 @@ cmd_url_save(ProfWin *window, const char *const command, gchar **args)
     }
 
     gchar *directory = NULL;
-    gchar *filename = NULL;
+    gchar *basename = NULL;
     if (path != NULL) {
         directory = g_path_get_dirname(path);
-        filename = g_path_get_basename(path);
+        basename = g_path_get_basename(path);
     }
 
+    if (directory == NULL) {
+        // Explicitly use "./" as directory if no directory has been passed.
+        directory = "./";
+    }
+
+    if (!g_file_test(directory, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
+        cons_show_error("Directory '%s' does not exist or is not a directory.", directory);
+        return TRUE;
+    }
+
+    if (!basename) {
+        basename = http_basename_from_url(url);
+    }
+
+    char *filename = NULL;
+    filename = g_build_filename(directory, basename, NULL);
+
+    char *unique_filename = _make_unique_filename(filename);
+    if (!unique_filename) {
+        cons_show_error("Failed to generate an unique filename from '%s'.", filename);
+        free(filename);
+        return TRUE;
+    }
+
+    free(filename);
+    filename = unique_filename;
+
     gchar *scheme_cmd = prefs_get_string_with_option(PREF_URL_SAVE_CMD, scheme);
     if (scheme_cmd == NULL) {
         if (g_strcmp0(scheme, "http") == 0
                 || g_strcmp0(scheme, "https") == 0
                 || g_strcmp0(scheme, OMEMO_AESGCM_URL_SCHEME) == 0) {
-            _url_save_fallback_method(window, url, directory, filename);
+            _url_save_fallback_method(window, url, filename);
         } else {
             cons_show_error("No download method defined for the scheme '%s'.", scheme);
         }
     } else {
-        _url_save_external_method(scheme_cmd, url, directory, filename);
+        _url_save_external_method(scheme_cmd, url, filename);
     }
 
     g_free(scheme_cmd);

src/tools/http_download.c

@@ -104,114 +104,6 @@ _older_progress(void *p, double dltotal, double dlnow, double ultotal, double ul
 }
 #endif
 
-char *http_filename_from_header(char *header) {
-    const char *header_tag_cd = "Content-Disposition:";
-    const int header_tag_cd_len = strlen(header_tag_cd);
-
-    if (!header) {
-        return NULL; // Bad header.
-    }
-
-    if (strncasecmp(header, header_tag_cd, header_tag_cd_len) == 0) {
-        header += header_tag_cd_len; // Move to header content.
-    } else {
-        return NULL; // Not a CD header.
-    }
-
-    const char *filename_key = "filename=";
-    const size_t filename_key_len = strlen(filename_key);
-
-    char *value = strcasestr(header, filename_key);
-    if (!value) {
-        return NULL; // No filename key found.
-    }
-
-    value += filename_key_len; // Move to key value.
-
-    char fn[4096];
-    char *pf = fn;
-    while(*value != '\0' && *value != ';') {
-        *pf++ = *value++;
-    }
-    *pf = '\0';
-
-    if (!strlen(fn)) {
-        return NULL; // Empty tag.
-    }
-
-    return strdup(fn);
-}
-
-char *http_filename_from_url(const char *url) {
-    const char *default_name = "index.html";
-
-    GFile *file = g_file_new_for_uri(url);
-    char *filename = g_file_get_basename(file);
-    g_object_unref(file);
-
-    if (g_strcmp0(filename, ".") == 0
-            || g_strcmp0(filename, G_DIR_SEPARATOR_S) == 0) {
-        g_free(filename);
-        return strdup(default_name);
-    }
-
-    return filename;
-}
-
-static size_t _header_callback(char *data, size_t size, size_t nitems, void *userdata) {
-    char *header = (char*)data;
-
-    HTTPDownload *download = (HTTPDownload *)userdata;
-    size *= nitems;
-
-    if (download->filename != NULL) {
-        return size; // No-op.
-    }
-
-    download->filename = http_filename_from_header(header);
-
-    return size;
-}
-
-FILE *_get_filehandle(const char *directory, const char *filename) {
-    gchar *fp;
-    FILE *fh;
-
-    // Explicitly use "." as directory if no directory has been passed.
-    if (directory == NULL) {
-        fp = g_build_filename(".", filename, NULL);
-    } else {
-        fp = g_build_filename(directory, filename, NULL);
-    }
-
-    fh = fopen(fp, "wb");
-    g_free(fp);
-    return fh;
-}
-
-static size_t _write_callback(void *buffer, size_t size, size_t nmemb, void *userdata) {
-    HTTPDownload *download = (HTTPDownload *)userdata;
-    size *= nmemb;
-
-    if (download->filename == NULL) {
-        download->filename = http_filename_from_url(download->url);
-    }
-
-    if (download->filename == NULL || download->directory == NULL) {
-        return 0; // Missing file name or directory, write no data.
-    }
-
-    if (download->filehandle == NULL ) {
-        FILE *fh = _get_filehandle(download->directory, download->filename);
-        if (!fh) {
-            return 0; // Unable to open file handle.
-        }
-        download->filehandle = fh;
-    }
-
-    return fwrite(buffer, size, nmemb, userdata);
-}
-
 void *
 http_file_get(void *userdata)
 {
@@ -250,11 +142,7 @@ http_file_get(void *userdata)
     #endif
     curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
 
-    curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, _header_callback);
+    curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)download->filehandle);
-    curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)download);
-
-    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, _write_callback);
-    curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)download);
 
     curl_easy_setopt(curl, CURLOPT_USERAGENT, "profanity");
 
@@ -306,8 +194,6 @@ http_file_get(void *userdata)
     pthread_mutex_unlock(&lock);
 
     free(download->url);
-    free(download->filename);
-    free(download->directory);
     free(download);
 
     return NULL;
@@ -332,3 +218,20 @@ http_download_add_download(HTTPDownload *download)
 {
     download_processes = g_slist_append(download_processes, download);
 }
+
+char *http_basename_from_url(const char *url) {
+    const char *default_name = "index.html";
+
+    GFile *file = g_file_new_for_uri(url);
+    char *filename = g_file_get_basename(file);
+    g_object_unref(file);
+
+    if (g_strcmp0(filename, ".") == 0
+            || g_strcmp0(filename, "..") == 0
+            || g_strcmp0(filename, G_DIR_SEPARATOR_S) == 0) {
+        g_free(filename);
+        return strdup(default_name);
+    }
+
+    return filename;
+}

src/tools/http_download.h

@@ -48,8 +48,6 @@
 
 typedef struct http_download_t {
     char *url;
-    char *filename;
-    char *directory;
     FILE *filehandle;
     curl_off_t bytes_received;
     ProfWin *window;
@@ -62,7 +60,6 @@ void* http_file_get(void *userdata);
 void http_download_cancel_processes(ProfWin *window);
 void http_download_add_download(HTTPDownload *download);
 
-char *http_filename_from_url(const char *url);
+char *http_basename_from_url(const char *url);
-char *http_filename_from_header(char *header);
 
 #endif

tests/unittests/test_http_download.c

@@ -12,108 +12,61 @@
 
 typedef struct {
     char *url;
-    char *filename;
+    char *basename;
 } url_test_t;
 
-typedef struct {
+void http_basename_from_url_td(void **state) {
-    char *header;
+    int num_tests = 11;
-    char *filename;
-} header_test_t;
-
-void http_filename_from_url_td(void **state) {
-    int num_tests = 5;
     url_test_t tests[] = {
       (url_test_t){
         .url = "https://host.test/image.jpeg",
-        .filename = "image.jpeg",
+        .basename = "image.jpeg",
+      },
+      (url_test_t){
+        .url = "https://host.test/image.jpeg#somefragment",
+        .basename = "image.jpeg",
+      },
+      (url_test_t){
+        .url = "https://host.test/image.jpeg?query=param",
+        .basename = "image.jpeg",
+      },
+      (url_test_t){
+        .url = "https://host.test/image.jpeg?query=param&another=one",
+        .basename = "image.jpeg",
       },
       (url_test_t){
         .url = "https://host.test/images/",
-        .filename = "images",
+        .basename = "images",
+      },
+      (url_test_t){
+        .url = "https://host.test/images/../../file",
+        .basename = "file",
+      },
+      (url_test_t){
+        .url = "https://host.test/images/../../file/..",
+        .basename = "index.html",
+      },
+      (url_test_t){
+        .url = "https://host.test/images/..//",
+        .basename = "index.html",
       },
       (url_test_t){
         .url = "https://host.test/",
-        .filename = "index.html",
+        .basename = "index.html",
       },
       (url_test_t){
         .url = "https://host.test",
-        .filename = "index.html",
+        .basename = "index.html",
       },
       (url_test_t){
         .url = "aesgcm://host.test",
-        .filename = "index.html",
+        .basename = "index.html",
       },
     };
 
-    char *filename;
+    char *basename;
     for(int i = 0; i < num_tests; i++) {
-      filename = http_filename_from_url(tests[i].url);
+        basename = http_basename_from_url(tests[i].url);
-      assert_string_equal(filename, tests[i].filename);
+        assert_string_equal(basename, tests[i].basename);
-    }
-}
-
-void http_filename_from_header_td(void **state) {
-    int num_tests = 11;
-    header_test_t tests[] = {
-      (header_test_t){
-        .header = "Content-Disposition: filename=image.jpeg",
-        .filename = "image.jpeg",
-      },
-      (header_test_t){
-        .header = "Content-Disposition:filename=image.jpeg",
-        .filename = "image.jpeg",
-      },
-      (header_test_t){
-        .header = "CoNteNt-DiSpoSItioN: filename=image.jpeg",
-        .filename = "image.jpeg",
-      },
-      (header_test_t){
-        .header = "Content-Disposition: attachment; filename=image.jpeg",
-        .filename = "image.jpeg",
-      },
-      (header_test_t){
-        .header = "Content-Disposition: filename=",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = "Content-Disposition: filename=;",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = "Content-Disposition: inline",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = "Content-Disposition:",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = "Last-Modified: Wed, 21 Oct 2015 07:28:00 GMT ",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = "",
-        .filename = NULL,
-      },
-      (header_test_t){
-        .header = NULL,
-        .filename = NULL,
-      },
-    };
-
-    char *got_filename;
-    char *exp_filename;
-    char *header;
-    for(int i = 0; i < num_tests; i++) {
-        header = tests[i].header;
-        exp_filename = tests[i].filename;
-
-        got_filename = http_filename_from_header(header);
-
-        if (exp_filename == NULL) {
-            assert_null(got_filename);
-        } else {
-            assert_string_equal(got_filename, exp_filename);
-        }
     }
 }

tests/unittests/test_http_download.h

@@ -1,2 +1 @@
-void http_filename_from_url_td(void **state);
+void http_basename_from_url_td(void **state);
-void http_filename_from_header_td(void **state);

tests/unittests/unittests.c

@@ -628,8 +628,7 @@ main(int argc, char* argv[])
         unit_test(removes_plugin_features),
         unit_test(does_not_remove_feature_when_more_than_one_reference),
 
-        unit_test(http_filename_from_url_td),
+        unit_test(http_basename_from_url_td),
-        unit_test(http_filename_from_header_td),
     };
 
     return run_tests(all_tests);