From 71f32ef2a8bc18c65e856609a68778db2b32930d Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Tue, 2 Aug 2022 16:38:49 -0700 Subject: [PATCH] Make sure we don't create comment options longer than 65535 bytes. Check in both editcap and Wireshark to make sure that comments have fewer than 65536 bytes before accepting them. This shoudl fix #18235, although there should also be checks in libwiretap to catch cases where the user interface code doesn't do the check (it should be done in the UI so that the user gets notified appropriately). --- editcap.c | 30 ++++++++++++++++++++++++ ui/qt/capture_file_properties_dialog.cpp | 15 ++++++++++++ ui/qt/packet_list.cpp | 27 ++++++++++++++++++++- 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/editcap.c b/editcap.c index 6738a25df5b..fb123bc521c 100644 --- a/editcap.c +++ b/editcap.c @@ -1284,6 +1284,21 @@ main(int argc, char *argv[]) case LONGOPT_CAPTURE_COMMENT: { + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (strlen(ws_optarg) > 65535) { + /* It doesn't fit. Tell the user and give up. */ + cmdarg_err("Capture comment %u is too large to save in a capture file.", + capture_comments->len + 1); + ret = INVALID_OPTION; + goto clean_exit; + } + /* pcapng supports multiple comments, so support them here too. */ if (!capture_comments) { @@ -1311,6 +1326,21 @@ main(int argc, char *argv[]) goto clean_exit; } + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (strlen(ws_optarg+string_start_index) > 65535) { + /* It doesn't fit. Tell the user and give up. */ + cmdarg_err("A comment for frame %u is too large to save in a capture file.", + frame_number); + ret = INVALID_OPTION; + goto clean_exit; + } + /* Lazily create the table */ if (!frames_user_comments) { frames_user_comments = g_tree_new_full(framenum_compare, NULL, NULL, g_free); diff --git a/ui/qt/capture_file_properties_dialog.cpp b/ui/qt/capture_file_properties_dialog.cpp index eb42bd9c331..f2213d909dc 100644 --- a/ui/qt/capture_file_properties_dialog.cpp +++ b/ui/qt/capture_file_properties_dialog.cpp @@ -12,6 +12,7 @@ #include "capture_file_properties_dialog.h" #include +#include "ui/simple_dialog.h" #include "ui/summary.h" #include "wsutil/str_util.h" @@ -616,6 +617,20 @@ void CaptureFilePropertiesDialog::on_buttonBox_accepted() if (wtap_dump_can_write(cap_file_.capFile()->linktypes, WTAP_COMMENT_PER_SECTION)) { gchar *str = qstring_strdup(ui->commentsTextEdit->toPlainText()); + + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (strlen(str) > 65535) { + /* It doesn't fit. Tell the user and give up. */ + simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, + "That coment is too large to save in a capture file."); + return; + } cf_update_section_comment(cap_file_.capFile(), str); emit captureCommentChanged(); fillDetails(); diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp index df01ed7245c..edb914957f4 100644 --- a/ui/qt/packet_list.cpp +++ b/ui/qt/packet_list.cpp @@ -32,6 +32,7 @@ #include "ui/recent.h" #include "ui/recent_utils.h" #include "ui/ws_ui_util.h" +#include "ui/simple_dialog.h" #include #include "ui/util.h" @@ -1415,6 +1416,18 @@ void PacketList::addPacketComment(QString new_comment) wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (ba.size() > 65535) { + simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, + "That coment is too large to save in a capture file."); + return; + } wtap_block_add_string_option(pkt_block, OPT_COMMENT, ba.data(), ba.size()); cf_set_modified_block(cap_file_, fdata, pkt_block); @@ -1441,6 +1454,18 @@ void PacketList::setPacketComment(guint c_number, QString new_comment) wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, c_number); } else { QByteArray ba = new_comment.toLocal8Bit(); + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (ba.size() > 65535) { + simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, + "That coment is too large to save in a capture file."); + return; + } wtap_block_set_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, ba.data(), ba.size()); } @@ -2132,4 +2157,4 @@ void PacketList::resizeAllColumns(bool onlyTimeFormatted) resizeColumnToContents(col); } } -} \ No newline at end of file +}