From a0cf0844abeeffe83a13e438eff3c309bc9887e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Wennerstr=C3=B6m?= Date: Sun, 5 Jul 2020 18:28:23 +0200 Subject: [PATCH] Remove unsafe Conent-Disposition inferring --- src/command/cmd_funcs.c | 99 ++++++++++++-------- src/tools/http_download.c | 133 ++++----------------------- src/tools/http_download.h | 5 +- tests/unittests/test_http_download.c | 117 +++++++---------------- tests/unittests/test_http_download.h | 3 +- tests/unittests/unittests.c | 3 +- 6 files changed, 117 insertions(+), 243 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 5e8b5cab..fb39f104 100644 --- a/src/command/cmd_funcs.c +++ b/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); - - if (filename) { - download->filename = strdup(filename); - } else { - download->filename = NULL; - } - - if (directory) { - download->directory = strdup(directory); - } else { - download->directory = NULL; - } + download->filehandle = fh; 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) { - 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; - } - +void _url_save_external_method(const char *scheme_cmd, const char *url, const char *filename) { 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); diff --git a/src/tools/http_download.c b/src/tools/http_download.c index 5a0f6f18..3d4003f9 100644 --- a/src/tools/http_download.c +++ b/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_HEADERDATA, (void *)download); - - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, _write_callback); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)download); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)download->filehandle); 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; +} diff --git a/src/tools/http_download.h b/src/tools/http_download.h index b0377d93..868b99f2 100644 --- a/src/tools/http_download.h +++ b/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_filename_from_header(char *header); +char *http_basename_from_url(const char *url); #endif diff --git a/tests/unittests/test_http_download.c b/tests/unittests/test_http_download.c index 181fc78d..c0516a66 100644 --- a/tests/unittests/test_http_download.c +++ b/tests/unittests/test_http_download.c @@ -12,108 +12,61 @@ typedef struct { char *url; - char *filename; + char *basename; } url_test_t; -typedef struct { - char *header; - char *filename; -} header_test_t; - -void http_filename_from_url_td(void **state) { - int num_tests = 5; +void http_basename_from_url_td(void **state) { + int num_tests = 11; 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); - assert_string_equal(filename, tests[i].filename); - } -} - -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); - } + basename = http_basename_from_url(tests[i].url); + assert_string_equal(basename, tests[i].basename); } } diff --git a/tests/unittests/test_http_download.h b/tests/unittests/test_http_download.h index c8c333ee..ce8913eb 100644 --- a/tests/unittests/test_http_download.h +++ b/tests/unittests/test_http_download.h @@ -1,2 +1 @@ -void http_filename_from_url_td(void **state); -void http_filename_from_header_td(void **state); +void http_basename_from_url_td(void **state); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index 9e9833bc..06c1b307 100644 --- a/tests/unittests/unittests.c +++ b/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_filename_from_header_td), + unit_test(http_basename_from_url_td), }; return run_tests(all_tests);