From e12270a730a60636ad36c06957b7e1f687c562a7 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Tue, 1 Jan 2019 13:04:44 -0800 Subject: [PATCH] Limit the maximum *file* name length for exported objects. Limiting the maximum *path* name length is bogus; if the user wants to store the file in some directory deep under the root (UN*X) or the root of the drive (Windows), that's their choice - don't prevent them from saving in a directory with a path longer than some maximum or limit the file name based on the length of the path leading up to it. Limiting the maximum *file* name is presumably to cope with, for example, HTTP objects with a URL that had a very long query component, so it makes sense. Change-Id: Idfc7de8124ee80bdd4950341ff2239834eb9f6f6 Reviewed-on: https://code.wireshark.org/review/31295 Petri-Dish: Guy Harris Tested-by: Petri Dish Buildbot Reviewed-by: Guy Harris --- epan/export_object.h | 4 ++ ui/cli/tap-exportobject.c | 54 ++++++++++++--------------- ui/qt/export_object_dialog.cpp | 4 +- ui/qt/models/export_objects_model.cpp | 6 +-- 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/epan/export_object.h b/epan/export_object.h index fd15a055c7..72e053571b 100644 --- a/epan/export_object.h +++ b/epan/export_object.h @@ -35,6 +35,10 @@ typedef struct _export_object_entry_t { guint8 *payload_data; } export_object_entry_t; +/** Maximum file name size for the file to which we save an object. + This is the file name size, not the path name size; we impose + the limit so that the file doesn't have a ridiculously long + name, e.g. an HTTP object where the URL has a long query part. */ #define EXPORT_OBJECT_MAXFILELEN 255 typedef void (*export_object_object_list_add_entry_cb)(void* gui_data, struct _export_object_entry_t *entry); diff --git a/ui/cli/tap-exportobject.c b/ui/cli/tap-exportobject.c index e804e19857..0766da779b 100644 --- a/ui/cli/tap-exportobject.c +++ b/ui/cli/tap-exportobject.c @@ -120,37 +120,31 @@ eo_draw(void *tapdata) } } - if ((strlen(save_in_path) < EXPORT_OBJECT_MAXFILELEN)) { - while (slist) { - entry = (export_object_entry_t *)slist->data; - do { - g_free(save_as_fullpath); - if (entry->filename) { - safe_filename = eo_massage_str(entry->filename, - EXPORT_OBJECT_MAXFILELEN - strlen(save_in_path), count); - } else { - char generic_name[EXPORT_OBJECT_MAXFILELEN+1]; - const char *ext; - ext = eo_ct2ext(entry->content_type); - g_snprintf(generic_name, sizeof(generic_name), - "object%u%s%s", entry->pkt_num, ext ? "." : "", ext ? ext : ""); - safe_filename = eo_massage_str(generic_name, - EXPORT_OBJECT_MAXFILELEN - strlen(save_in_path), count); - } - save_as_fullpath = g_build_filename(save_in_path, safe_filename->str, NULL); - g_string_free(safe_filename, TRUE); - } while (g_file_test(save_as_fullpath, G_FILE_TEST_EXISTS) && ++count < 1000); - count = 0; - if (!eo_save_entry(save_as_fullpath, entry, TRUE)) - all_saved = FALSE; + while (slist) { + entry = (export_object_entry_t *)slist->data; + do { g_free(save_as_fullpath); - save_as_fullpath = NULL; - slist = slist->next; - } - } - else - { - all_saved = FALSE; + if (entry->filename) { + safe_filename = eo_massage_str(entry->filename, + EXPORT_OBJECT_MAXFILELEN, count); + } else { + char generic_name[EXPORT_OBJECT_MAXFILELEN+1]; + const char *ext; + ext = eo_ct2ext(entry->content_type); + g_snprintf(generic_name, sizeof(generic_name), + "object%u%s%s", entry->pkt_num, ext ? "." : "", ext ? ext : ""); + safe_filename = eo_massage_str(generic_name, + EXPORT_OBJECT_MAXFILELEN, count); + } + save_as_fullpath = g_build_filename(save_in_path, safe_filename->str, NULL); + g_string_free(safe_filename, TRUE); + } while (g_file_test(save_as_fullpath, G_FILE_TEST_EXISTS) && ++count < 1000); + count = 0; + if (!eo_save_entry(save_as_fullpath, entry, TRUE)) + all_saved = FALSE; + g_free(save_as_fullpath); + save_as_fullpath = NULL; + slist = slist->next; } if (!all_saved) diff --git a/ui/qt/export_object_dialog.cpp b/ui/qt/export_object_dialog.cpp index a9bbbf7055..44b7b3b806 100644 --- a/ui/qt/export_object_dialog.cpp +++ b/ui/qt/export_object_dialog.cpp @@ -164,7 +164,7 @@ void ExportObjectDialog::saveCurrentEntry() if (entry_filename.isEmpty()) return; - GString *safe_filename = eo_massage_str(entry_filename.toUtf8().constData(), EXPORT_OBJECT_MAXFILELEN-path.canonicalPath().length(), 0); + GString *safe_filename = eo_massage_str(entry_filename.toUtf8().constData(), EXPORT_OBJECT_MAXFILELEN, 0); QString file_name = WiresharkFileDialog::getSaveFileName(this, wsApp->windowTitleString(tr("Save Object As" UTF8_HORIZONTAL_ELLIPSIS)), safe_filename->str); g_string_free(safe_filename, TRUE); @@ -191,7 +191,7 @@ void ExportObjectDialog::saveAllEntries() save_in_dir.canonicalPath(), QFileDialog::ShowDirsOnly); - if (save_in_path.length() < 1 || save_in_path.length() > EXPORT_OBJECT_MAXFILELEN) + if (save_in_path.length() < 1) return; if (!model_.saveAllEntries(save_in_path)) diff --git a/ui/qt/models/export_objects_model.cpp b/ui/qt/models/export_objects_model.cpp index cade148a24..a1d96b6624 100644 --- a/ui/qt/models/export_objects_model.cpp +++ b/ui/qt/models/export_objects_model.cpp @@ -175,16 +175,16 @@ bool ExportObjectModel::saveAllEntries(QString path) g_free(save_as_fullpath); if (entry->filename) safe_filename = eo_massage_str(entry->filename, - EXPORT_OBJECT_MAXFILELEN - path.length(), count); + EXPORT_OBJECT_MAXFILELEN, count); else { - char generic_name[256]; + char generic_name[EXPORT_OBJECT_MAXFILELEN+1]; const char *ext; ext = eo_ct2ext(entry->content_type); g_snprintf(generic_name, sizeof(generic_name), "object%u%s%s", entry->pkt_num, ext ? "." : "", ext ? ext : ""); safe_filename = eo_massage_str(generic_name, - EXPORT_OBJECT_MAXFILELEN - path.length(), count); + EXPORT_OBJECT_MAXFILELEN, count); } save_as_fullpath = g_build_filename(path.toUtf8().constData(), safe_filename->str, NULL);